| Message ID | 1460064119-21282-1-git-send-email-bill.fischofer@linaro.org |
|---|---|
| State | New |
| Headers | show |
Should we add something to the user-guide at the same time ? We had said a few times we wanted new apis to go beyond just having a unit test for inclusion, but that they also must come with documentation, that of course also helps the reviewer understand the use case for the new code. On 7 April 2016 at 17:21, Bill Fischofer <bill.fischofer@linaro.org> wrote: > 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.Pricope@enea.com> > Signed-off-by: Bill Fischofer <bill.fischofer@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 > https://lists.linaro.org/mailman/listinfo/lng-odp > -- Mike Holmes Technical Manager - Linaro Networking Group Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs "Work should be fun and collaborative, the rest follows"
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.savolainen@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-bounces@lists.linaro.org] On Behalf Of EXT > > Bill Fischofer > > Sent: Friday, April 08, 2016 12:22 AM > > To: 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.Pricope@enea.com> > > Signed-off-by: Bill Fischofer <bill.fischofer@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 > > https://lists.linaro.org/mailman/listinfo/lng-odp >
Agree, Mike, however the user guide is supposed to cover concepts and usage models rather than individual APIs. I'd rather see a general section on context management that discusses this in a more comprehensive manner and touches on all the facilities of ODP that support user contexts. It's on my to-do list. On Fri, Apr 8, 2016 at 6:41 AM, Mike Holmes <mike.holmes@linaro.org> wrote: > Should we add something to the user-guide at the same time ? > > We had said a few times we wanted new apis to go beyond just having a unit > test for inclusion, but that they also must come with documentation, that > of course also helps the reviewer understand the use case for the new code. > > > On 7 April 2016 at 17:21, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> 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.Pricope@enea.com> >> Signed-off-by: Bill Fischofer <bill.fischofer@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 >> https://lists.linaro.org/mailman/listinfo/lng-odp >> > > > > -- > Mike Holmes > Technical Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs > "Work should be fun and collaborative, the rest follows" > > >
On 8 April 2016 at 17:25, Savolainen, Petri (Nokia - FI/Espoo) < petri.savolainen@nokia.com> wrote: > > > > > *From:* EXT Bogdan Pricope [mailto:Bogdan.Pricope@enea.com] > *Sent:* Friday, April 08, 2016 4:50 PM > *To:* Bill Fischofer <bill.fischofer@linaro.org>; Savolainen, Petri > (Nokia - FI/Espoo) <petri.savolainen@nokia.com> > *Cc:* lng-odp@lists.linaro.org > *Subject:* RE: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user > context support for interfaces > > > > Hi, > > > > Our path is: > > pktio = odp_packet_input(pkt); > > A step is invisible here: > > queue = odp_pktio_outq_getdef(pktio) > > ifnet = ofp_queue_context(queue) > > > > ifnet = ofp_get_ifnet_pktio(pktio); > > > > (Our current workaround is to iterate through the list of interfaces to > find which interface has this pktio.) > > > > You are saying: > > pktio_id = odp_packet_input_id(pkt); > > ifnet = ofp_get_ifnet_pktio_id(pktio_id); > > > > Code generated with ID: > > pktio_id = pkt_hdr->pktio_id; > > ifnet = &ofp_ifnet_table[pktio_id]: > I think a pktio integer identifier (defined as a small integer suitable for using as an array index) is the most generic solution. Different SW layers can use this pktio ID as an index into *different* tables (different SW layers will have different needs). A (single) ODP pktio context pointer only makes one SW layer happy (or will promote a monolithic SW design which will be fragile and difficult to extend with new functionality). -- 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.fischofer@linaro.org > <bill.fischofer@linaro.org>] > *Sent:* Friday, April 08, 2016 3:05 PM > *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; > Bogdan Pricope <Bogdan.Pricope@enea.com> > *Cc:* 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.savolainen@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-bounces@lists.linaro.org] On Behalf Of EXT > > Bill Fischofer > > Sent: Friday, April 08, 2016 12:22 AM > > To: 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.Pricope@enea.com> > > Signed-off-by: Bill Fischofer <bill.fischofer@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 > > 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 > >
On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > On 8 April 2016 at 17:25, Savolainen, Petri (Nokia - FI/Espoo) < > petri.savolainen@nokia.com> wrote: > >> >> >> >> >> *From:* EXT Bogdan Pricope [mailto:Bogdan.Pricope@enea.com] >> *Sent:* Friday, April 08, 2016 4:50 PM >> *To:* Bill Fischofer <bill.fischofer@linaro.org>; Savolainen, Petri >> (Nokia - FI/Espoo) <petri.savolainen@nokia.com> >> *Cc:* lng-odp@lists.linaro.org >> *Subject:* RE: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user >> context support for interfaces >> >> >> >> Hi, >> >> >> >> Our path is: >> >> pktio = odp_packet_input(pkt); >> >> A step is invisible here: >> >> queue = odp_pktio_outq_getdef(pktio) >> >> ifnet = ofp_queue_context(queue) >> >> >> >> ifnet = ofp_get_ifnet_pktio(pktio); >> >> >> >> (Our current workaround is to iterate through the list of interfaces to >> find which interface has this pktio.) >> >> >> >> You are saying: >> >> pktio_id = odp_packet_input_id(pkt); >> >> ifnet = ofp_get_ifnet_pktio_id(pktio_id); >> >> >> >> Code generated with ID: >> >> pktio_id = pkt_hdr->pktio_id; >> >> ifnet = &ofp_ifnet_table[pktio_id]: >> > I think a pktio integer identifier (defined as a small integer suitable > for using as an array index) is the most generic solution. Different SW > layers can use this pktio ID as an index into *different* tables (different > SW layers will have different needs). A (single) ODP pktio context pointer > only makes one SW layer happy (or will promote a monolithic SW design which > will be fragile and difficult to extend with new functionality). > > That sounds reasonable. So should we consider the following new APIs? uint32_t odp_pktio_to_id(odp_pktio_t pktio); This returns a small integer index in the range 0..num_pktios-1 odp_pktio_t odp_pktio_from_id(uint32_t id); 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.fischofer@linaro.org >> <bill.fischofer@linaro.org>] >> *Sent:* Friday, April 08, 2016 3:05 PM >> *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; >> Bogdan Pricope <Bogdan.Pricope@enea.com> >> *Cc:* 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.savolainen@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-bounces@lists.linaro.org] On Behalf Of >> EXT >> > Bill Fischofer >> > Sent: Friday, April 08, 2016 12:22 AM >> > To: 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.Pricope@enea.com> >> > Signed-off-by: Bill Fischofer <bill.fischofer@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 >> > 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 >> >> >
On Wed, Apr 13, 2016 at 12:36 PM, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: > >> On 8 April 2016 at 17:25, Savolainen, Petri (Nokia - FI/Espoo) < >> petri.savolainen@nokia.com> wrote: >> >>> >>> >>> >>> >>> *From:* EXT Bogdan Pricope [mailto:Bogdan.Pricope@enea.com] >>> *Sent:* Friday, April 08, 2016 4:50 PM >>> *To:* Bill Fischofer <bill.fischofer@linaro.org>; Savolainen, Petri >>> (Nokia - FI/Espoo) <petri.savolainen@nokia.com> >>> *Cc:* lng-odp@lists.linaro.org >>> *Subject:* RE: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user >>> context support for interfaces >>> >>> >>> >>> Hi, >>> >>> >>> >>> Our path is: >>> >>> pktio = odp_packet_input(pkt); >>> >>> A step is invisible here: >>> >>> queue = odp_pktio_outq_getdef(pktio) >>> >>> ifnet = ofp_queue_context(queue) >>> >>> >>> >>> ifnet = ofp_get_ifnet_pktio(pktio); >>> >>> >>> >>> (Our current workaround is to iterate through the list of interfaces to >>> find which interface has this pktio.) >>> >>> >>> >>> You are saying: >>> >>> pktio_id = odp_packet_input_id(pkt); >>> >>> ifnet = ofp_get_ifnet_pktio_id(pktio_id); >>> >>> >>> >>> Code generated with ID: >>> >>> pktio_id = pkt_hdr->pktio_id; >>> >>> ifnet = &ofp_ifnet_table[pktio_id]: >>> >> I think a pktio integer identifier (defined as a small integer suitable >> for using as an array index) is the most generic solution. Different SW >> layers can use this pktio ID as an index into *different* tables (different >> SW layers will have different needs). A (single) ODP pktio context pointer >> only makes one SW layer happy (or will promote a monolithic SW design which >> will be fragile and difficult to extend with new functionality). >> >> > That sounds reasonable. So should we consider the following new APIs? > > 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.fischofer@linaro.org >>> <bill.fischofer@linaro.org>] >>> *Sent:* Friday, April 08, 2016 3:05 PM >>> *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; >>> Bogdan Pricope <Bogdan.Pricope@enea.com> >>> *Cc:* 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.savolainen@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-bounces@lists.linaro.org] On Behalf Of >>> EXT >>> > Bill Fischofer >>> > Sent: Friday, April 08, 2016 12:22 AM >>> > To: 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.Pricope@enea.com> >>> > Signed-off-by: Bill Fischofer <bill.fischofer@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 >>> > 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 >>> >>> >> >
I've posted a patch series (starting http://patches.opendataplane.org/patch/5556/) for these. Bogdan: Please confirm that this works for you. On Wed, Apr 13, 2016 at 12:38 PM, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > > On Wed, Apr 13, 2016 at 12:36 PM, Bill Fischofer < > bill.fischofer@linaro.org> wrote: > >> >> On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl <ola.liljedahl@linaro.org> >> wrote: >> >>> On 8 April 2016 at 17:25, Savolainen, Petri (Nokia - FI/Espoo) < >>> petri.savolainen@nokia.com> wrote: >>> >>>> >>>> >>>> >>>> >>>> *From:* EXT Bogdan Pricope [mailto:Bogdan.Pricope@enea.com] >>>> *Sent:* Friday, April 08, 2016 4:50 PM >>>> *To:* Bill Fischofer <bill.fischofer@linaro.org>; Savolainen, Petri >>>> (Nokia - FI/Espoo) <petri.savolainen@nokia.com> >>>> *Cc:* lng-odp@lists.linaro.org >>>> *Subject:* RE: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user >>>> context support for interfaces >>>> >>>> >>>> >>>> Hi, >>>> >>>> >>>> >>>> Our path is: >>>> >>>> pktio = odp_packet_input(pkt); >>>> >>>> A step is invisible here: >>>> >>>> queue = odp_pktio_outq_getdef(pktio) >>>> >>>> ifnet = ofp_queue_context(queue) >>>> >>>> >>>> >>>> ifnet = ofp_get_ifnet_pktio(pktio); >>>> >>>> >>>> >>>> (Our current workaround is to iterate through the list of interfaces to >>>> find which interface has this pktio.) >>>> >>>> >>>> >>>> You are saying: >>>> >>>> pktio_id = odp_packet_input_id(pkt); >>>> >>>> ifnet = ofp_get_ifnet_pktio_id(pktio_id); >>>> >>>> >>>> >>>> Code generated with ID: >>>> >>>> pktio_id = pkt_hdr->pktio_id; >>>> >>>> ifnet = &ofp_ifnet_table[pktio_id]: >>>> >>> I think a pktio integer identifier (defined as a small integer suitable >>> for using as an array index) is the most generic solution. Different SW >>> layers can use this pktio ID as an index into *different* tables (different >>> SW layers will have different needs). A (single) ODP pktio context pointer >>> only makes one SW layer happy (or will promote a monolithic SW design which >>> will be fragile and difficult to extend with new functionality). >>> >>> >> That sounds reasonable. So should we consider the following new APIs? >> >> 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.fischofer@linaro.org >>>> <bill.fischofer@linaro.org>] >>>> *Sent:* Friday, April 08, 2016 3:05 PM >>>> *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; >>>> Bogdan Pricope <Bogdan.Pricope@enea.com> >>>> *Cc:* 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.savolainen@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-bounces@lists.linaro.org] On Behalf Of >>>> EXT >>>> > Bill Fischofer >>>> > Sent: Friday, April 08, 2016 12:22 AM >>>> > To: 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.Pricope@enea.com> >>>> > Signed-off-by: Bill Fischofer <bill.fischofer@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 >>>> > 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 >>>> >>>> >>> >> >
On 13 April 2016 at 19:38, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > > On Wed, Apr 13, 2016 at 12:36 PM, Bill Fischofer < > bill.fischofer@linaro.org> wrote: > >> >> On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl <ola.liljedahl@linaro.org> >> wrote: >> >>> On 8 April 2016 at 17:25, Savolainen, Petri (Nokia - FI/Espoo) < >>> petri.savolainen@nokia.com> wrote: >>> >>>> >>>> >>>> >>>> >>>> *From:* EXT Bogdan Pricope [mailto:Bogdan.Pricope@enea.com] >>>> *Sent:* Friday, April 08, 2016 4:50 PM >>>> *To:* Bill Fischofer <bill.fischofer@linaro.org>; Savolainen, Petri >>>> (Nokia - FI/Espoo) <petri.savolainen@nokia.com> >>>> *Cc:* lng-odp@lists.linaro.org >>>> *Subject:* RE: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user >>>> context support for interfaces >>>> >>>> >>>> >>>> Hi, >>>> >>>> >>>> >>>> Our path is: >>>> >>>> pktio = odp_packet_input(pkt); >>>> >>>> A step is invisible here: >>>> >>>> queue = odp_pktio_outq_getdef(pktio) >>>> >>>> ifnet = ofp_queue_context(queue) >>>> >>>> >>>> >>>> ifnet = ofp_get_ifnet_pktio(pktio); >>>> >>>> >>>> >>>> (Our current workaround is to iterate through the list of interfaces to >>>> find which interface has this pktio.) >>>> >>>> >>>> >>>> You are saying: >>>> >>>> pktio_id = odp_packet_input_id(pkt); >>>> >>>> ifnet = ofp_get_ifnet_pktio_id(pktio_id); >>>> >>>> >>>> >>>> Code generated with ID: >>>> >>>> pktio_id = pkt_hdr->pktio_id; >>>> >>>> ifnet = &ofp_ifnet_table[pktio_id]: >>>> >>> I think a pktio integer identifier (defined as a small integer suitable >>> for using as an array index) is the most generic solution. Different SW >>> layers can use this pktio ID as an index into *different* tables (different >>> SW layers will have different needs). A (single) ODP pktio context pointer >>> only makes one SW layer happy (or will promote a monolithic SW design which >>> will be fragile and difficult to extend with new functionality). >>> >>> >> That sounds reasonable. So should we consider the following new APIs? >> > Yes looks good to me except "id" in the name feels a bit short and not specific enough. What about "pktioid"? (well that doesn't look great either, I usually call this thing "ifindex" for interface index). But we also want an API to extract the pktio ID from a packet (which identifies which pktio the packet was received on). int odp_packet_pktioid(odp_packet_t pkt); Will return -1 for a packet that was not received on some interface (i.e. returned from odp_packet_alloc()). > >> uint32_t odp_pktio_to_id(odp_pktio_t pktio); >> >> This returns a small integer index in the range 0..num_pktios-1 >> > > Actually that first signature should be int rather than uint32_t since -1 > would be returned for failure. > > >> >> odp_pktio_t odp_pktio_from_id(uint32_t id); >> > > For symmetry id should be typed as int rather than uint32_t? > > >> >> Returns the pktio handle 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.fischofer@linaro.org >>>> <bill.fischofer@linaro.org>] >>>> *Sent:* Friday, April 08, 2016 3:05 PM >>>> *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; >>>> Bogdan Pricope <Bogdan.Pricope@enea.com> >>>> *Cc:* 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.savolainen@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-bounces@lists.linaro.org] On Behalf Of >>>> EXT >>>> > Bill Fischofer >>>> > Sent: Friday, April 08, 2016 12:22 AM >>>> > To: 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.Pricope@enea.com> >>>> > Signed-off-by: Bill Fischofer <bill.fischofer@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 >>>> > 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 >>>> >>>> >>> >> >
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? Id can be renamed index if that's the preference. On Wed, Apr 13, 2016 at 2:22 PM, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > > > On 13 April 2016 at 19:38, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> >> >> On Wed, Apr 13, 2016 at 12:36 PM, Bill Fischofer < >> bill.fischofer@linaro.org> wrote: >> >>> >>> On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl <ola.liljedahl@linaro.org >>> > wrote: >>> >>>> On 8 April 2016 at 17:25, Savolainen, Petri (Nokia - FI/Espoo) < >>>> petri.savolainen@nokia.com> wrote: >>>> >>>>> >>>>> >>>>> >>>>> >>>>> *From:* EXT Bogdan Pricope [mailto:Bogdan.Pricope@enea.com] >>>>> *Sent:* Friday, April 08, 2016 4:50 PM >>>>> *To:* Bill Fischofer <bill.fischofer@linaro.org>; Savolainen, Petri >>>>> (Nokia - FI/Espoo) <petri.savolainen@nokia.com> >>>>> *Cc:* lng-odp@lists.linaro.org >>>>> *Subject:* RE: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user >>>>> context support for interfaces >>>>> >>>>> >>>>> >>>>> Hi, >>>>> >>>>> >>>>> >>>>> Our path is: >>>>> >>>>> pktio = odp_packet_input(pkt); >>>>> >>>>> A step is invisible here: >>>>> >>>>> queue = odp_pktio_outq_getdef(pktio) >>>>> >>>>> ifnet = ofp_queue_context(queue) >>>>> >>>>> >>>>> >>>>> ifnet = ofp_get_ifnet_pktio(pktio); >>>>> >>>>> >>>>> >>>>> (Our current workaround is to iterate through the list of interfaces >>>>> to find which interface has this pktio.) >>>>> >>>>> >>>>> >>>>> You are saying: >>>>> >>>>> pktio_id = odp_packet_input_id(pkt); >>>>> >>>>> ifnet = ofp_get_ifnet_pktio_id(pktio_id); >>>>> >>>>> >>>>> >>>>> Code generated with ID: >>>>> >>>>> pktio_id = pkt_hdr->pktio_id; >>>>> >>>>> ifnet = &ofp_ifnet_table[pktio_id]: >>>>> >>>> I think a pktio integer identifier (defined as a small integer suitable >>>> for using as an array index) is the most generic solution. Different SW >>>> layers can use this pktio ID as an index into *different* tables (different >>>> SW layers will have different needs). A (single) ODP pktio context pointer >>>> only makes one SW layer happy (or will promote a monolithic SW design which >>>> will be fragile and difficult to extend with new functionality). >>>> >>>> >>> That sounds reasonable. So should we consider the following new APIs? >>> >> Yes looks good to me except "id" in the name feels a bit short and not > specific enough. What about "pktioid"? (well that doesn't look great > either, I usually call this thing "ifindex" for interface index). > > But we also want an API to extract the pktio ID from a packet (which > identifies which pktio the packet was received on). > int odp_packet_pktioid(odp_packet_t pkt); > Will return -1 for a packet that was not received on some interface (i.e. > returned from odp_packet_alloc()). > > >> >>> uint32_t odp_pktio_to_id(odp_pktio_t pktio); >>> >>> This returns a small integer index in the range 0..num_pktios-1 >>> >> >> Actually that first signature should be int rather than uint32_t since -1 >> would be returned for failure. >> >> >>> >>> odp_pktio_t odp_pktio_from_id(uint32_t id); >>> >> >> For symmetry id should be typed as int rather than uint32_t? >> >> >>> >>> Returns the pktio handle 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.fischofer@linaro.org >>>>> <bill.fischofer@linaro.org>] >>>>> *Sent:* Friday, April 08, 2016 3:05 PM >>>>> *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; >>>>> Bogdan Pricope <Bogdan.Pricope@enea.com> >>>>> *Cc:* 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.savolainen@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-bounces@lists.linaro.org] On Behalf >>>>> Of EXT >>>>> > Bill Fischofer >>>>> > Sent: Friday, April 08, 2016 12:22 AM >>>>> > To: 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.Pricope@enea.com> >>>>> > Signed-off-by: Bill Fischofer <bill.fischofer@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 >>>>> > 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 >>>>> >>>>> >>>> >>> >> >
On 13 April 2016 at 21:26, Bill Fischofer <bill.fischofer@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. > Id can be renamed index if that's the preference. > > On Wed, Apr 13, 2016 at 2:22 PM, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: > >> >> >> On 13 April 2016 at 19:38, Bill Fischofer <bill.fischofer@linaro.org> >> wrote: >> >>> >>> >>> On Wed, Apr 13, 2016 at 12:36 PM, Bill Fischofer < >>> bill.fischofer@linaro.org> wrote: >>> >>>> >>>> On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl < >>>> ola.liljedahl@linaro.org> wrote: >>>> >>>>> On 8 April 2016 at 17:25, Savolainen, Petri (Nokia - FI/Espoo) < >>>>> petri.savolainen@nokia.com> wrote: >>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> *From:* EXT Bogdan Pricope [mailto:Bogdan.Pricope@enea.com] >>>>>> *Sent:* Friday, April 08, 2016 4:50 PM >>>>>> *To:* Bill Fischofer <bill.fischofer@linaro.org>; Savolainen, Petri >>>>>> (Nokia - FI/Espoo) <petri.savolainen@nokia.com> >>>>>> *Cc:* lng-odp@lists.linaro.org >>>>>> *Subject:* RE: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user >>>>>> context support for interfaces >>>>>> >>>>>> >>>>>> >>>>>> Hi, >>>>>> >>>>>> >>>>>> >>>>>> Our path is: >>>>>> >>>>>> pktio = odp_packet_input(pkt); >>>>>> >>>>>> A step is invisible here: >>>>>> >>>>>> queue = odp_pktio_outq_getdef(pktio) >>>>>> >>>>>> ifnet = ofp_queue_context(queue) >>>>>> >>>>>> >>>>>> >>>>>> ifnet = ofp_get_ifnet_pktio(pktio); >>>>>> >>>>>> >>>>>> >>>>>> (Our current workaround is to iterate through the list of interfaces >>>>>> to find which interface has this pktio.) >>>>>> >>>>>> >>>>>> >>>>>> You are saying: >>>>>> >>>>>> pktio_id = odp_packet_input_id(pkt); >>>>>> >>>>>> ifnet = ofp_get_ifnet_pktio_id(pktio_id); >>>>>> >>>>>> >>>>>> >>>>>> Code generated with ID: >>>>>> >>>>>> pktio_id = pkt_hdr->pktio_id; >>>>>> >>>>>> ifnet = &ofp_ifnet_table[pktio_id]: >>>>>> >>>>> I think a pktio integer identifier (defined as a small integer >>>>> suitable for using as an array index) is the most generic solution. >>>>> Different SW layers can use this pktio ID as an index into *different* >>>>> tables (different SW layers will have different needs). A (single) ODP >>>>> pktio context pointer only makes one SW layer happy (or will promote a >>>>> monolithic SW design which will be fragile and difficult to extend with new >>>>> functionality). >>>>> >>>>> >>>> That sounds reasonable. So should we consider the following new APIs? >>>> >>> Yes looks good to me except "id" in the name feels a bit short and not >> specific enough. What about "pktioid"? (well that doesn't look great >> either, I usually call this thing "ifindex" for interface index). >> >> But we also want an API to extract the pktio ID from a packet (which >> identifies which pktio the packet was received on). >> int odp_packet_pktioid(odp_packet_t pkt); >> Will return -1 for a packet that was not received on some interface (i.e. >> returned from odp_packet_alloc()). >> >> >>> >>>> uint32_t odp_pktio_to_id(odp_pktio_t pktio); >>>> >>>> This returns a small integer index in the range 0..num_pktios-1 >>>> >>> >>> Actually that first signature should be int rather than uint32_t since >>> -1 would be returned for failure. >>> >>> >>>> >>>> odp_pktio_t odp_pktio_from_id(uint32_t id); >>>> >>> >>> For symmetry id should be typed as int rather than uint32_t? >>> >>> >>>> >>>> Returns the pktio handle 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.fischofer@linaro.org >>>>>> <bill.fischofer@linaro.org>] >>>>>> *Sent:* Friday, April 08, 2016 3:05 PM >>>>>> *To:* Savolainen, Petri (Nokia - FI/Espoo) < >>>>>> petri.savolainen@nokia.com>; Bogdan Pricope <Bogdan.Pricope@enea.com> >>>>>> *Cc:* 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.savolainen@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-bounces@lists.linaro.org] On Behalf >>>>>> Of EXT >>>>>> > Bill Fischofer >>>>>> > Sent: Friday, April 08, 2016 12:22 AM >>>>>> > To: 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.Pricope@enea.com> >>>>>> > Signed-off-by: Bill Fischofer <bill.fischofer@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 >>>>>> > 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 >>>>>> >>>>>> >>>>> >>>> >>> >> >
On Wed, Apr 13, 2016 at 2:40 PM, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > > > On 13 April 2016 at 21:26, Bill Fischofer <bill.fischofer@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.liljedahl@linaro.org> >> wrote: >> >>> >>> >>> On 13 April 2016 at 19:38, Bill Fischofer <bill.fischofer@linaro.org> >>> wrote: >>> >>>> >>>> >>>> On Wed, Apr 13, 2016 at 12:36 PM, Bill Fischofer < >>>> bill.fischofer@linaro.org> wrote: >>>> >>>>> >>>>> On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl < >>>>> ola.liljedahl@linaro.org> wrote: >>>>> >>>>>> On 8 April 2016 at 17:25, Savolainen, Petri (Nokia - FI/Espoo) < >>>>>> petri.savolainen@nokia.com> wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> *From:* EXT Bogdan Pricope [mailto:Bogdan.Pricope@enea.com] >>>>>>> *Sent:* Friday, April 08, 2016 4:50 PM >>>>>>> *To:* Bill Fischofer <bill.fischofer@linaro.org>; Savolainen, Petri >>>>>>> (Nokia - FI/Espoo) <petri.savolainen@nokia.com> >>>>>>> *Cc:* lng-odp@lists.linaro.org >>>>>>> *Subject:* RE: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user >>>>>>> context support for interfaces >>>>>>> >>>>>>> >>>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> >>>>>>> >>>>>>> Our path is: >>>>>>> >>>>>>> pktio = odp_packet_input(pkt); >>>>>>> >>>>>>> A step is invisible here: >>>>>>> >>>>>>> queue = odp_pktio_outq_getdef(pktio) >>>>>>> >>>>>>> ifnet = ofp_queue_context(queue) >>>>>>> >>>>>>> >>>>>>> >>>>>>> ifnet = ofp_get_ifnet_pktio(pktio); >>>>>>> >>>>>>> >>>>>>> >>>>>>> (Our current workaround is to iterate through the list of interfaces >>>>>>> to find which interface has this pktio.) >>>>>>> >>>>>>> >>>>>>> >>>>>>> You are saying: >>>>>>> >>>>>>> pktio_id = odp_packet_input_id(pkt); >>>>>>> >>>>>>> ifnet = ofp_get_ifnet_pktio_id(pktio_id); >>>>>>> >>>>>>> >>>>>>> >>>>>>> Code generated with ID: >>>>>>> >>>>>>> pktio_id = pkt_hdr->pktio_id; >>>>>>> >>>>>>> ifnet = &ofp_ifnet_table[pktio_id]: >>>>>>> >>>>>> I think a pktio integer identifier (defined as a small integer >>>>>> suitable for using as an array index) is the most generic solution. >>>>>> Different SW layers can use this pktio ID as an index into *different* >>>>>> tables (different SW layers will have different needs). A (single) ODP >>>>>> pktio context pointer only makes one SW layer happy (or will promote a >>>>>> monolithic SW design which will be fragile and difficult to extend with new >>>>>> functionality). >>>>>> >>>>>> >>>>> That sounds reasonable. So should we consider the following new APIs? >>>>> >>>> Yes looks good to me except "id" in the name feels a bit short and not >>> specific enough. What about "pktioid"? (well that doesn't look great >>> either, I usually call this thing "ifindex" for interface index). >>> >>> But we also want an API to extract the pktio ID from a packet (which >>> identifies which pktio the packet was received on). >>> int odp_packet_pktioid(odp_packet_t pkt); >>> Will return -1 for a packet that was not received on some interface >>> (i.e. returned from odp_packet_alloc()). >>> >>> >>>> >>>>> uint32_t odp_pktio_to_id(odp_pktio_t pktio); >>>>> >>>>> This returns a small integer index in the range 0..num_pktios-1 >>>>> >>>> >>>> Actually that first signature should be int rather than uint32_t since >>>> -1 would be returned for failure. >>>> >>>> >>>>> >>>>> odp_pktio_t odp_pktio_from_id(uint32_t id); >>>>> >>>> >>>> For symmetry id should be typed as int rather than uint32_t? >>>> >>>> >>>>> >>>>> Returns the pktio handle 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.fischofer@linaro.org >>>>>>> <bill.fischofer@linaro.org>] >>>>>>> *Sent:* Friday, April 08, 2016 3:05 PM >>>>>>> *To:* Savolainen, Petri (Nokia - FI/Espoo) < >>>>>>> petri.savolainen@nokia.com>; Bogdan Pricope <Bogdan.Pricope@enea.com >>>>>>> > >>>>>>> *Cc:* 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.savolainen@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-bounces@lists.linaro.org] On Behalf >>>>>>> Of EXT >>>>>>> > Bill Fischofer >>>>>>> > Sent: Friday, April 08, 2016 12:22 AM >>>>>>> > To: 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.Pricope@enea.com> >>>>>>> > Signed-off-by: Bill Fischofer <bill.fischofer@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 >>>>>>> > 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 >>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
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.Pricope@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.fischofer@linaro.org] > *Sent:* Wednesday, April 13, 2016 10:47 PM > *To:* Ola Liljedahl <ola.liljedahl@linaro.org> > *Cc:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; > Bogdan Pricope <Bogdan.Pricope@enea.com>; lng-odp@lists.linaro.org > > *Subject:* Re: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user > context support for interfaces > > > > > > On Wed, Apr 13, 2016 at 2:40 PM, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: > > > > > > On 13 April 2016 at 21:26, Bill Fischofer <bill.fischofer@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.liljedahl@linaro.org> > wrote: > > > > > > On 13 April 2016 at 19:38, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > > > > > > On Wed, Apr 13, 2016 at 12:36 PM, Bill Fischofer < > bill.fischofer@linaro.org> wrote: > > > > On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: > > On 8 April 2016 at 17:25, Savolainen, Petri (Nokia - FI/Espoo) < > petri.savolainen@nokia.com> wrote: > > > > > > *From:* EXT Bogdan Pricope [mailto:Bogdan.Pricope@enea.com] > *Sent:* Friday, April 08, 2016 4:50 PM > *To:* Bill Fischofer <bill.fischofer@linaro.org>; Savolainen, Petri > (Nokia - FI/Espoo) <petri.savolainen@nokia.com> > *Cc:* lng-odp@lists.linaro.org > *Subject:* RE: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user > context support for interfaces > > > > Hi, > > > > Our path is: > > pktio = odp_packet_input(pkt); > > A step is invisible here: > > queue = odp_pktio_outq_getdef(pktio) > > ifnet = ofp_queue_context(queue) > > > > ifnet = ofp_get_ifnet_pktio(pktio); > > > > (Our current workaround is to iterate through the list of interfaces to > find which interface has this pktio.) > > > > You are saying: > > pktio_id = odp_packet_input_id(pkt); > > ifnet = ofp_get_ifnet_pktio_id(pktio_id); > > > > Code generated with ID: > > pktio_id = pkt_hdr->pktio_id; > > ifnet = &ofp_ifnet_table[pktio_id]: > > I think a pktio integer identifier (defined as a small integer suitable > for using as an array index) is the most generic solution. Different SW > layers can use this pktio ID as an index into *different* tables (different > SW layers will have different needs). A (single) ODP pktio context pointer > only makes one SW layer happy (or will promote a monolithic SW design which > will be fragile and difficult to extend with new functionality). > > > > > > That sounds reasonable. So should we consider the following new APIs? > > Yes looks good to me except "id" in the name feels a bit short and not > specific enough. What about "pktioid"? (well that doesn't look great > either, I usually call this thing "ifindex" for interface index). > > > > But we also want an API to extract the pktio ID from a packet (which > identifies which pktio the packet was received on). > > int odp_packet_pktioid(odp_packet_t pkt); > > Will return -1 for a packet that was not received on some interface (i.e. > returned from odp_packet_alloc()). > > > > > > uint32_t odp_pktio_to_id(odp_pktio_t pktio); > > > > This returns a small integer index in the range 0..num_pktios-1 > > > > Actually that first signature should be int rather than uint32_t since -1 > would be returned for failure. > > > > > > odp_pktio_t odp_pktio_from_id(uint32_t id); > > > > For symmetry id should be typed as int rather than uint32_t? > > > > > > Returns the pktio handle 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.fischofer@linaro.org > <bill.fischofer@linaro.org>] > *Sent:* Friday, April 08, 2016 3:05 PM > *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; > Bogdan Pricope <Bogdan.Pricope@enea.com> > *Cc:* 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.savolainen@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-bounces@lists.linaro.org] On Behalf Of EXT > > Bill Fischofer > > Sent: Friday, April 08, 2016 12:22 AM > > To: 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.Pricope@enea.com> > > Signed-off-by: Bill Fischofer <bill.fischofer@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 > > 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 > > > > > > > > > > > > > > >
Hi Bill, Yes, it should work (with some effort) fine with OFP. Reviewed-by: Bogdan Pricope <Bogdan.Pricope@enea.com> Thanks, Bogdan From: Bill Fischofer [mailto:bill.fischofer@linaro.org] Sent: Friday, April 15, 2016 12:16 AM To: Bogdan Pricope <Bogdan.Pricope@enea.com> Cc: Ola Liljedahl <ola.liljedahl@linaro.org>; Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@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.Pricope@enea.com<mailto:Bogdan.Pricope@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.fischofer@linaro.org<mailto:bill.fischofer@linaro.org>] Sent: Wednesday, April 13, 2016 10:47 PM To: Ola Liljedahl <ola.liljedahl@linaro.org<mailto:ola.liljedahl@linaro.org>> Cc: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com<mailto:petri.savolainen@nokia.com>>; Bogdan Pricope <Bogdan.Pricope@enea.com<mailto:Bogdan.Pricope@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.liljedahl@linaro.org<mailto:ola.liljedahl@linaro.org>> wrote: On 13 April 2016 at 21:26, Bill Fischofer <bill.fischofer@linaro.org<mailto:bill.fischofer@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.liljedahl@linaro.org<mailto:ola.liljedahl@linaro.org>> wrote: On 13 April 2016 at 19:38, Bill Fischofer <bill.fischofer@linaro.org<mailto:bill.fischofer@linaro.org>> wrote: On Wed, Apr 13, 2016 at 12:36 PM, Bill Fischofer <bill.fischofer@linaro.org<mailto:bill.fischofer@linaro.org>> wrote: On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl <ola.liljedahl@linaro.org<mailto:ola.liljedahl@linaro.org>> wrote: On 8 April 2016 at 17:25, Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com<mailto:petri.savolainen@nokia.com>> wrote: From: EXT Bogdan Pricope [mailto:Bogdan.Pricope@enea.com<mailto:Bogdan.Pricope@enea.com>] Sent: Friday, April 08, 2016 4:50 PM To: Bill Fischofer <bill.fischofer@linaro.org<mailto:bill.fischofer@linaro.org>>; Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com<mailto:petri.savolainen@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.fischofer@linaro.org] Sent: Friday, April 08, 2016 3:05 PM To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com<mailto:petri.savolainen@nokia.com>>; Bogdan Pricope <Bogdan.Pricope@enea.com<mailto:Bogdan.Pricope@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.savolainen@nokia.com<mailto:petri.savolainen@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-bounces@lists.linaro.org<mailto:lng-odp-bounces@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.Pricope@enea.com<mailto:Bogdan.Pricope@enea.com>> > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org<mailto:bill.fischofer@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
On 04/15/16 08:56, Bogdan Pricope wrote: > > Hi Bill, > > Yes, it should work (with some effort) fine with OFP. > > Reviewed-by: Bogdan Pricope <Bogdan.Pricope@enea.com> > just to clarify, is that review for current patchset or for [lng-odp] [API-NEXT PATCHv2 1/6] api: pktio: add pktio index conversion APIs ? Maxim. > Thanks, > > Bogdan > > *From:*Bill Fischofer [mailto:bill.fischofer@linaro.org] > *Sent:* Friday, April 15, 2016 12:16 AM > *To:* Bogdan Pricope <Bogdan.Pricope@enea.com> > *Cc:* Ola Liljedahl <ola.liljedahl@linaro.org>; Savolainen, Petri > (Nokia - FI/Espoo) <petri.savolainen@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.Pricope@enea.com <mailto:Bogdan.Pricope@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.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>] > *Sent:* Wednesday, April 13, 2016 10:47 PM > *To:* Ola Liljedahl <ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>> > *Cc:* Savolainen, Petri (Nokia - FI/Espoo) > <petri.savolainen@nokia.com <mailto:petri.savolainen@nokia.com>>; > Bogdan Pricope <Bogdan.Pricope@enea.com > <mailto:Bogdan.Pricope@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.liljedahl@linaro.org <mailto:ola.liljedahl@linaro.org>> wrote: > > On 13 April 2016 at 21:26, Bill Fischofer > <bill.fischofer@linaro.org <mailto:bill.fischofer@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.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>> wrote: > > On 13 April 2016 at 19:38, Bill Fischofer > <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>> wrote: > > On Wed, Apr 13, 2016 at 12:36 PM, Bill Fischofer > <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>> wrote: > > On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl > <ola.liljedahl@linaro.org > <mailto:ola.liljedahl@linaro.org>> wrote: > > On 8 April 2016 at 17:25, Savolainen, > Petri (Nokia - FI/Espoo) > <petri.savolainen@nokia.com > <mailto:petri.savolainen@nokia.com>> wrote: > > *From:*EXT Bogdan Pricope > [mailto:Bogdan.Pricope@enea.com > <mailto:Bogdan.Pricope@enea.com>] > *Sent:* Friday, April 08, 2016 4:50 PM > *To:* Bill Fischofer > <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>>; > Savolainen, Petri (Nokia - FI/Espoo) > <petri.savolainen@nokia.com > <mailto:petri.savolainen@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.fischofer@linaro.org] > *Sent:* Friday, April 08, 2016 3:05 PM > *To:* Savolainen, Petri (Nokia - > FI/Espoo) <petri.savolainen@nokia.com > <mailto:petri.savolainen@nokia.com>>; > Bogdan Pricope > <Bogdan.Pricope@enea.com > <mailto:Bogdan.Pricope@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.savolainen@nokia.com > <mailto:petri.savolainen@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-bounces@lists.linaro.org > <mailto:lng-odp-bounces@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.Pricope@enea.com > <mailto:Bogdan.Pricope@enea.com>> > > Signed-off-by: Bill Fischofer > <bill.fischofer@linaro.org > <mailto:bill.fischofer@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
Yes, however Petri has some suggested naming changes. I'll post a v3 with those and add Bogdan's review to it. On Fri, Apr 15, 2016 at 1:25 AM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 04/15/16 08:56, Bogdan Pricope wrote: > >> >> Hi Bill, >> >> Yes, it should work (with some effort) fine with OFP. >> >> Reviewed-by: Bogdan Pricope <Bogdan.Pricope@enea.com> >> >> > just to clarify, is that review for current patchset or for [lng-odp] > [API-NEXT PATCHv2 1/6] api: pktio: add pktio index conversion APIs ? > > Maxim. > > Thanks, >> >> Bogdan >> >> *From:*Bill Fischofer [mailto:bill.fischofer@linaro.org] >> *Sent:* Friday, April 15, 2016 12:16 AM >> *To:* Bogdan Pricope <Bogdan.Pricope@enea.com> >> *Cc:* Ola Liljedahl <ola.liljedahl@linaro.org>; Savolainen, Petri (Nokia >> - FI/Espoo) <petri.savolainen@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.Pricope@enea.com >> <mailto:Bogdan.Pricope@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.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org>] >> *Sent:* Wednesday, April 13, 2016 10:47 PM >> *To:* Ola Liljedahl <ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org>> >> *Cc:* Savolainen, Petri (Nokia - FI/Espoo) >> <petri.savolainen@nokia.com <mailto:petri.savolainen@nokia.com>>; >> Bogdan Pricope <Bogdan.Pricope@enea.com >> <mailto:Bogdan.Pricope@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.liljedahl@linaro.org <mailto:ola.liljedahl@linaro.org>> wrote: >> >> On 13 April 2016 at 21:26, Bill Fischofer >> <bill.fischofer@linaro.org <mailto:bill.fischofer@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.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org>> wrote: >> >> On 13 April 2016 at 19:38, Bill Fischofer >> <bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org>> wrote: >> >> On Wed, Apr 13, 2016 at 12:36 PM, Bill Fischofer >> <bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org>> wrote: >> >> On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl >> <ola.liljedahl@linaro.org >> <mailto:ola.liljedahl@linaro.org>> wrote: >> >> On 8 April 2016 at 17:25, Savolainen, >> Petri (Nokia - FI/Espoo) >> <petri.savolainen@nokia.com >> <mailto:petri.savolainen@nokia.com>> wrote: >> >> *From:*EXT Bogdan Pricope >> [mailto:Bogdan.Pricope@enea.com >> <mailto:Bogdan.Pricope@enea.com>] >> *Sent:* Friday, April 08, 2016 4:50 PM >> *To:* Bill Fischofer >> <bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org>>; >> Savolainen, Petri (Nokia - FI/Espoo) >> <petri.savolainen@nokia.com >> <mailto:petri.savolainen@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.fischofer@linaro.org] >> *Sent:* Friday, April 08, 2016 3:05 PM >> *To:* Savolainen, Petri (Nokia - >> FI/Espoo) <petri.savolainen@nokia.com >> <mailto:petri.savolainen@nokia.com>>; >> Bogdan Pricope >> <Bogdan.Pricope@enea.com >> <mailto:Bogdan.Pricope@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.savolainen@nokia.com >> <mailto:petri.savolainen@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-bounces@lists.linaro.org >> <mailto: >> lng-odp-bounces@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.Pricope@enea.com >> <mailto:Bogdan.Pricope@enea.com>> >> > Signed-off-by: Bill Fischofer >> <bill.fischofer@linaro.org >> <mailto:bill.fischofer@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 >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
On 13 April 2016 at 21:47, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > On Wed, Apr 13, 2016 at 2:40 PM, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: > >> >> >> On 13 April 2016 at 21:26, Bill Fischofer <bill.fischofer@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? > I don't think we in practice can depend on being able to classify packets into flows on the required granularity. Flows will be relatively coarse-grained. Which interface a packet was received on is a pretty basic piece of metadata. The current odp_packet_input() just isn't good enough for many of the potential use cases for ingress interface identifier. > > >> >> >>> Id can be renamed index if that's the preference. >>> >>> On Wed, Apr 13, 2016 at 2:22 PM, Ola Liljedahl <ola.liljedahl@linaro.org >>> > wrote: >>> >>>> >>>> >>>> On 13 April 2016 at 19:38, Bill Fischofer <bill.fischofer@linaro.org> >>>> wrote: >>>> >>>>> >>>>> >>>>> On Wed, Apr 13, 2016 at 12:36 PM, Bill Fischofer < >>>>> bill.fischofer@linaro.org> wrote: >>>>> >>>>>> >>>>>> On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl < >>>>>> ola.liljedahl@linaro.org> wrote: >>>>>> >>>>>>> On 8 April 2016 at 17:25, Savolainen, Petri (Nokia - FI/Espoo) < >>>>>>> petri.savolainen@nokia.com> wrote: >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> *From:* EXT Bogdan Pricope [mailto:Bogdan.Pricope@enea.com] >>>>>>>> *Sent:* Friday, April 08, 2016 4:50 PM >>>>>>>> *To:* Bill Fischofer <bill.fischofer@linaro.org>; Savolainen, >>>>>>>> Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com> >>>>>>>> *Cc:* lng-odp@lists.linaro.org >>>>>>>> *Subject:* RE: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user >>>>>>>> context support for interfaces >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Our path is: >>>>>>>> >>>>>>>> pktio = odp_packet_input(pkt); >>>>>>>> >>>>>>>> A step is invisible here: >>>>>>>> >>>>>>>> queue = odp_pktio_outq_getdef(pktio) >>>>>>>> >>>>>>>> ifnet = ofp_queue_context(queue) >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> ifnet = ofp_get_ifnet_pktio(pktio); >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> (Our current workaround is to iterate through the list of >>>>>>>> interfaces to find which interface has this pktio.) >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> You are saying: >>>>>>>> >>>>>>>> pktio_id = odp_packet_input_id(pkt); >>>>>>>> >>>>>>>> ifnet = ofp_get_ifnet_pktio_id(pktio_id); >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Code generated with ID: >>>>>>>> >>>>>>>> pktio_id = pkt_hdr->pktio_id; >>>>>>>> >>>>>>>> ifnet = &ofp_ifnet_table[pktio_id]: >>>>>>>> >>>>>>> I think a pktio integer identifier (defined as a small integer >>>>>>> suitable for using as an array index) is the most generic solution. >>>>>>> Different SW layers can use this pktio ID as an index into *different* >>>>>>> tables (different SW layers will have different needs). A (single) ODP >>>>>>> pktio context pointer only makes one SW layer happy (or will promote a >>>>>>> monolithic SW design which will be fragile and difficult to extend with new >>>>>>> functionality). >>>>>>> >>>>>>> >>>>>> That sounds reasonable. So should we consider the following new APIs? >>>>>> >>>>> Yes looks good to me except "id" in the name feels a bit short and not >>>> specific enough. What about "pktioid"? (well that doesn't look great >>>> either, I usually call this thing "ifindex" for interface index). >>>> >>>> But we also want an API to extract the pktio ID from a packet (which >>>> identifies which pktio the packet was received on). >>>> int odp_packet_pktioid(odp_packet_t pkt); >>>> Will return -1 for a packet that was not received on some interface >>>> (i.e. returned from odp_packet_alloc()). >>>> >>>> >>>>> >>>>>> uint32_t odp_pktio_to_id(odp_pktio_t pktio); >>>>>> >>>>>> This returns a small integer index in the range 0..num_pktios-1 >>>>>> >>>>> >>>>> Actually that first signature should be int rather than uint32_t since >>>>> -1 would be returned for failure. >>>>> >>>>> >>>>>> >>>>>> odp_pktio_t odp_pktio_from_id(uint32_t id); >>>>>> >>>>> >>>>> For symmetry id should be typed as int rather than uint32_t? >>>>> >>>>> >>>>>> >>>>>> Returns the pktio handle 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.fischofer@linaro.org >>>>>>>> <bill.fischofer@linaro.org>] >>>>>>>> *Sent:* Friday, April 08, 2016 3:05 PM >>>>>>>> *To:* Savolainen, Petri (Nokia - FI/Espoo) < >>>>>>>> petri.savolainen@nokia.com>; Bogdan Pricope < >>>>>>>> Bogdan.Pricope@enea.com> >>>>>>>> *Cc:* 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.savolainen@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-bounces@lists.linaro.org] On >>>>>>>> Behalf Of EXT >>>>>>>> > Bill Fischofer >>>>>>>> > Sent: Friday, April 08, 2016 12:22 AM >>>>>>>> > To: 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.Pricope@enea.com> >>>>>>>> > Signed-off-by: Bill Fischofer <bill.fischofer@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 >>>>>>>> > 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/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
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.Pricope@enea.com> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> --- include/odp/api/spec/packet_io.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)