diff mbox series

[1/2] linux-gen: ipc: use hdr to handle conversion function

Message ID 1488211057-15651-1-git-send-email-petri.savolainen@linaro.org
State Superseded
Headers show
Series [1/2] linux-gen: ipc: use hdr to handle conversion function | expand

Commit Message

Petri Savolainen Feb. 27, 2017, 3:57 p.m. UTC
Use conversion function instead of casting.

Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

---
 platform/linux-generic/include/odp_packet_internal.h | 6 ++++++
 platform/linux-generic/odp_packet.c                  | 5 -----
 platform/linux-generic/pktio/ipc.c                   | 2 +-
 3 files changed, 7 insertions(+), 6 deletions(-)

-- 
2.8.1

Comments

Bill Fischofer Feb. 27, 2017, 4:02 p.m. UTC | #1
On Mon, Feb 27, 2017 at 9:57 AM, Petri Savolainen <
petri.savolainen@linaro.org> wrote:

> Use conversion function instead of casting.

>

> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

> ---

>  platform/linux-generic/include/odp_packet_internal.h | 6 ++++++

>  platform/linux-generic/odp_packet.c                  | 5 -----

>  platform/linux-generic/pktio/ipc.c                   | 2 +-

>  3 files changed, 7 insertions(+), 6 deletions(-)

>

> diff --git a/platform/linux-generic/include/odp_packet_internal.h

> b/platform/linux-generic/include/odp_packet_internal.h

> index 4f844d1..7a394dd 100644

> --- a/platform/linux-generic/include/odp_packet_internal.h

> +++ b/platform/linux-generic/include/odp_packet_internal.h

> @@ -163,6 +163,12 @@ static inline odp_packet_hdr_t

> *odp_packet_hdr(odp_packet_t pkt)

>         return (odp_packet_hdr_t *)(uintptr_t)pkt;

>  }

>

> +/* Only one using this outside of packet.c is ipc.c */

>


I'd delete this comment but otherwise this series looks good. Who else is
using this function is potentially anyone who includes packet_internal.h
and that will change over time.


> +static inline odp_packet_t packet_handle(odp_packet_hdr_t *pkt_hdr)

> +{

> +       return (odp_packet_t)pkt_hdr;

> +}

> +

>  static inline void copy_packet_parser_metadata(odp_packet_hdr_t *src_hdr,

>                                                odp_packet_hdr_t *dst_hdr)

>  {

> diff --git a/platform/linux-generic/odp_packet.c

> b/platform/linux-generic/odp_packet.c

> index c21f635..3019418 100644

> --- a/platform/linux-generic/odp_packet.c

> +++ b/platform/linux-generic/odp_packet.c

> @@ -48,11 +48,6 @@ static inline odp_packet_hdr_t *packet_hdr(odp_packet_t

> pkt)

>         return (odp_packet_hdr_t *)(uintptr_t)pkt;

>  }

>

> -static inline odp_packet_t packet_handle(odp_packet_hdr_t *pkt_hdr)

> -{

> -       return (odp_packet_t)pkt_hdr;

> -}

> -

>  static inline odp_buffer_t buffer_handle(odp_packet_hdr_t *pkt_hdr)

>  {

>         return pkt_hdr->buf_hdr.handle.handle;

> diff --git a/platform/linux-generic/pktio/ipc.c b/platform/linux-generic/

> pktio/ipc.c

> index 377f20e..06175e5 100644

> --- a/platform/linux-generic/pktio/ipc.c

> +++ b/platform/linux-generic/pktio/ipc.c

> @@ -409,7 +409,7 @@ static void _ipc_free_ring_packets(pktio_entry_t

> *pktio_entry, _ring_t *r)

>                         void *mbase = pktio_entry->s.ipc.pool_mdata_base;

>

>                         phdr = (void *)((uint8_t *)mbase + offsets[i]);

> -                       pkt = (odp_packet_t)phdr->buf_hdr.handle.handle;

> +                       pkt = packet_handle(phdr);

>                         odp_packet_free(pkt);

>                 }

>         }

> --

> 2.8.1

>

>
Maxim Uvarov Feb. 27, 2017, 4:09 p.m. UTC | #2
On 02/27/17 19:02, Bill Fischofer wrote:
> On Mon, Feb 27, 2017 at 9:57 AM, Petri Savolainen <

> petri.savolainen@linaro.org> wrote:

> 

>> Use conversion function instead of casting.

>>

>> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

>> ---

>>  platform/linux-generic/include/odp_packet_internal.h | 6 ++++++

>>  platform/linux-generic/odp_packet.c                  | 5 -----

>>  platform/linux-generic/pktio/ipc.c                   | 2 +-

>>  3 files changed, 7 insertions(+), 6 deletions(-)

>>

>> diff --git a/platform/linux-generic/include/odp_packet_internal.h

>> b/platform/linux-generic/include/odp_packet_internal.h

>> index 4f844d1..7a394dd 100644

>> --- a/platform/linux-generic/include/odp_packet_internal.h

>> +++ b/platform/linux-generic/include/odp_packet_internal.h

>> @@ -163,6 +163,12 @@ static inline odp_packet_hdr_t

>> *odp_packet_hdr(odp_packet_t pkt)

>>         return (odp_packet_hdr_t *)(uintptr_t)pkt;

>>  }

>>

>> +/* Only one using this outside of packet.c is ipc.c */

>>

> 

> I'd delete this comment but otherwise this series looks good. Who else is

> using this function is potentially anyone who includes packet_internal.h

> and that will change over time.

> 


+1 to remove.

Maxim.

> 

>> +static inline odp_packet_t packet_handle(odp_packet_hdr_t *pkt_hdr)

>> +{

>> +       return (odp_packet_t)pkt_hdr;

>> +}

>> +

>>  static inline void copy_packet_parser_metadata(odp_packet_hdr_t *src_hdr,

>>                                                odp_packet_hdr_t *dst_hdr)

>>  {

>> diff --git a/platform/linux-generic/odp_packet.c

>> b/platform/linux-generic/odp_packet.c

>> index c21f635..3019418 100644

>> --- a/platform/linux-generic/odp_packet.c

>> +++ b/platform/linux-generic/odp_packet.c

>> @@ -48,11 +48,6 @@ static inline odp_packet_hdr_t *packet_hdr(odp_packet_t

>> pkt)

>>         return (odp_packet_hdr_t *)(uintptr_t)pkt;

>>  }

>>

>> -static inline odp_packet_t packet_handle(odp_packet_hdr_t *pkt_hdr)

>> -{

>> -       return (odp_packet_t)pkt_hdr;

>> -}

>> -

>>  static inline odp_buffer_t buffer_handle(odp_packet_hdr_t *pkt_hdr)

>>  {

>>         return pkt_hdr->buf_hdr.handle.handle;

>> diff --git a/platform/linux-generic/pktio/ipc.c b/platform/linux-generic/

>> pktio/ipc.c

>> index 377f20e..06175e5 100644

>> --- a/platform/linux-generic/pktio/ipc.c

>> +++ b/platform/linux-generic/pktio/ipc.c

>> @@ -409,7 +409,7 @@ static void _ipc_free_ring_packets(pktio_entry_t

>> *pktio_entry, _ring_t *r)

>>                         void *mbase = pktio_entry->s.ipc.pool_mdata_base;

>>

>>                         phdr = (void *)((uint8_t *)mbase + offsets[i]);

>> -                       pkt = (odp_packet_t)phdr->buf_hdr.handle.handle;

>> +                       pkt = packet_handle(phdr);

>>                         odp_packet_free(pkt);

>>                 }

>>         }

>> --

>> 2.8.1

>>

>>
Savolainen, Petri (Nokia - FI/Espoo) Feb. 28, 2017, 7:45 a.m. UTC | #3
> -----Original Message-----

> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of Maxim

> Uvarov

> Sent: Monday, February 27, 2017 6:10 PM

> To: lng-odp@lists.linaro.org

> Subject: Re: [lng-odp] [PATCH 1/2] linux-gen: ipc: use hdr to handle

> conversion function

> 

> On 02/27/17 19:02, Bill Fischofer wrote:

> > On Mon, Feb 27, 2017 at 9:57 AM, Petri Savolainen <

> > petri.savolainen@linaro.org> wrote:

> >

> >> Use conversion function instead of casting.

> >>

> >> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

> >> ---

> >>  platform/linux-generic/include/odp_packet_internal.h | 6 ++++++

> >>  platform/linux-generic/odp_packet.c                  | 5 -----

> >>  platform/linux-generic/pktio/ipc.c                   | 2 +-

> >>  3 files changed, 7 insertions(+), 6 deletions(-)

> >>

> >> diff --git a/platform/linux-generic/include/odp_packet_internal.h

> >> b/platform/linux-generic/include/odp_packet_internal.h

> >> index 4f844d1..7a394dd 100644

> >> --- a/platform/linux-generic/include/odp_packet_internal.h

> >> +++ b/platform/linux-generic/include/odp_packet_internal.h

> >> @@ -163,6 +163,12 @@ static inline odp_packet_hdr_t

> >> *odp_packet_hdr(odp_packet_t pkt)

> >>         return (odp_packet_hdr_t *)(uintptr_t)pkt;

> >>  }

> >>

> >> +/* Only one using this outside of packet.c is ipc.c */

> >>

> >

> > I'd delete this comment but otherwise this series looks good. Who else

> is

> > using this function is potentially anyone who includes packet_internal.h

> > and that will change over time.

> >

> 

> +1 to remove.

> 

> Maxim.


It's there to remind that maybe IPC should not have this dependency either. All other pktios manage to do their job without this conversion. So, I'm not expecting anyone adding this call outside of packet.c, but instead ipc.c to remove the call after which we can move the function back into packet.c.

-Petri
Bill Fischofer Feb. 28, 2017, 12:55 p.m. UTC | #4
On Tue, Feb 28, 2017 at 1:45 AM, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolainen@nokia-bell-labs.com> wrote:

>

>

> > -----Original Message-----

> > From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

> Maxim

> > Uvarov

> > Sent: Monday, February 27, 2017 6:10 PM

> > To: lng-odp@lists.linaro.org

> > Subject: Re: [lng-odp] [PATCH 1/2] linux-gen: ipc: use hdr to handle

> > conversion function

> >

> > On 02/27/17 19:02, Bill Fischofer wrote:

> > > On Mon, Feb 27, 2017 at 9:57 AM, Petri Savolainen <

> > > petri.savolainen@linaro.org> wrote:

> > >

> > >> Use conversion function instead of casting.

> > >>

> > >> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

> > >> ---

> > >>  platform/linux-generic/include/odp_packet_internal.h | 6 ++++++

> > >>  platform/linux-generic/odp_packet.c                  | 5 -----

> > >>  platform/linux-generic/pktio/ipc.c                   | 2 +-

> > >>  3 files changed, 7 insertions(+), 6 deletions(-)

> > >>

> > >> diff --git a/platform/linux-generic/include/odp_packet_internal.h

> > >> b/platform/linux-generic/include/odp_packet_internal.h

> > >> index 4f844d1..7a394dd 100644

> > >> --- a/platform/linux-generic/include/odp_packet_internal.h

> > >> +++ b/platform/linux-generic/include/odp_packet_internal.h

> > >> @@ -163,6 +163,12 @@ static inline odp_packet_hdr_t

> > >> *odp_packet_hdr(odp_packet_t pkt)

> > >>         return (odp_packet_hdr_t *)(uintptr_t)pkt;

> > >>  }

> > >>

> > >> +/* Only one using this outside of packet.c is ipc.c */

> > >>

> > >

> > > I'd delete this comment but otherwise this series looks good. Who else

> > is

> > > using this function is potentially anyone who includes

> packet_internal.h

> > > and that will change over time.

> > >

> >

> > +1 to remove.

> >

> > Maxim.

>

> It's there to remind that maybe IPC should not have this dependency

> either. All other pktios manage to do their job without this conversion.

> So, I'm not expecting anyone adding this call outside of packet.c, but

> instead ipc.c to remove the call after which we can move the function back

> into packet.c.

>


If ipc can operate without that conversion then it can be converted now,
but my understanding is that because it operates between ODP instances it's
something of a special case. I'll add this topic to today's ODP call.


>

> -Petri

>
Bill Fischofer Feb. 28, 2017, 12:58 p.m. UTC | #5
Your series also includes TM (good catch) so it seems this need extends
beyond just IPC.

On Tue, Feb 28, 2017 at 6:55 AM, Bill Fischofer <bill.fischofer@linaro.org>
wrote:

>

>

> On Tue, Feb 28, 2017 at 1:45 AM, Savolainen, Petri (Nokia - FI/Espoo) <

> petri.savolainen@nokia-bell-labs.com> wrote:

>

>>

>>

>> > -----Original Message-----

>> > From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>> Maxim

>> > Uvarov

>> > Sent: Monday, February 27, 2017 6:10 PM

>> > To: lng-odp@lists.linaro.org

>> > Subject: Re: [lng-odp] [PATCH 1/2] linux-gen: ipc: use hdr to handle

>> > conversion function

>> >

>> > On 02/27/17 19:02, Bill Fischofer wrote:

>> > > On Mon, Feb 27, 2017 at 9:57 AM, Petri Savolainen <

>> > > petri.savolainen@linaro.org> wrote:

>> > >

>> > >> Use conversion function instead of casting.

>> > >>

>> > >> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

>> > >> ---

>> > >>  platform/linux-generic/include/odp_packet_internal.h | 6 ++++++

>> > >>  platform/linux-generic/odp_packet.c                  | 5 -----

>> > >>  platform/linux-generic/pktio/ipc.c                   | 2 +-

>> > >>  3 files changed, 7 insertions(+), 6 deletions(-)

>> > >>

>> > >> diff --git a/platform/linux-generic/include/odp_packet_internal.h

>> > >> b/platform/linux-generic/include/odp_packet_internal.h

>> > >> index 4f844d1..7a394dd 100644

>> > >> --- a/platform/linux-generic/include/odp_packet_internal.h

>> > >> +++ b/platform/linux-generic/include/odp_packet_internal.h

>> > >> @@ -163,6 +163,12 @@ static inline odp_packet_hdr_t

>> > >> *odp_packet_hdr(odp_packet_t pkt)

>> > >>         return (odp_packet_hdr_t *)(uintptr_t)pkt;

>> > >>  }

>> > >>

>> > >> +/* Only one using this outside of packet.c is ipc.c */

>> > >>

>> > >

>> > > I'd delete this comment but otherwise this series looks good. Who else

>> > is

>> > > using this function is potentially anyone who includes

>> packet_internal.h

>> > > and that will change over time.

>> > >

>> >

>> > +1 to remove.

>> >

>> > Maxim.

>>

>> It's there to remind that maybe IPC should not have this dependency

>> either. All other pktios manage to do their job without this conversion.

>> So, I'm not expecting anyone adding this call outside of packet.c, but

>> instead ipc.c to remove the call after which we can move the function back

>> into packet.c.

>>

>

> If ipc can operate without that conversion then it can be converted now,

> but my understanding is that because it operates between ODP instances it's

> something of a special case. I'll add this topic to today's ODP call.

>

>

>>

>> -Petri

>>

>

>
Maxim Uvarov Feb. 28, 2017, 1:15 p.m. UTC | #6
On 02/28/17 15:55, Bill Fischofer wrote:
> 

> 

> On Tue, Feb 28, 2017 at 1:45 AM, Savolainen, Petri (Nokia - FI/Espoo)

> <petri.savolainen@nokia-bell-labs.com

> <mailto:petri.savolainen@nokia-bell-labs.com>> wrote:

> 

> 

> 

>     > -----Original Message-----

>     > From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org

>     <mailto:lng-odp-bounces@lists.linaro.org>] On Behalf Of Maxim

>     > Uvarov

>     > Sent: Monday, February 27, 2017 6:10 PM

>     > To: lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>

>     > Subject: Re: [lng-odp] [PATCH 1/2] linux-gen: ipc: use hdr to handle

>     > conversion function

>     >

>     > On 02/27/17 19:02, Bill Fischofer wrote:

>     > > On Mon, Feb 27, 2017 at 9:57 AM, Petri Savolainen <

>     > > petri.savolainen@linaro.org

>     <mailto:petri.savolainen@linaro.org>> wrote:

>     > >

>     > >> Use conversion function instead of casting.

>     > >>

>     > >> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org

>     <mailto:petri.savolainen@linaro.org>>

>     > >> ---

>     > >>  platform/linux-generic/include/odp_packet_internal.h | 6 ++++++

>     > >>  platform/linux-generic/odp_packet.c                  | 5 -----

>     > >>  platform/linux-generic/pktio/ipc.c                   | 2 +-

>     > >>  3 files changed, 7 insertions(+), 6 deletions(-)

>     > >>

>     > >> diff --git a/platform/linux-generic/include/odp_packet_internal.h

>     > >> b/platform/linux-generic/include/odp_packet_internal.h

>     > >> index 4f844d1..7a394dd 100644

>     > >> --- a/platform/linux-generic/include/odp_packet_internal.h

>     > >> +++ b/platform/linux-generic/include/odp_packet_internal.h

>     > >> @@ -163,6 +163,12 @@ static inline odp_packet_hdr_t

>     > >> *odp_packet_hdr(odp_packet_t pkt)

>     > >>         return (odp_packet_hdr_t *)(uintptr_t)pkt;

>     > >>  }

>     > >>

>     > >> +/* Only one using this outside of packet.c is ipc.c */

>     > >>

>     > >

>     > > I'd delete this comment but otherwise this series looks good.

>     Who else

>     > is

>     > > using this function is potentially anyone who includes

>     packet_internal.h

>     > > and that will change over time.

>     > >

>     >

>     > +1 to remove.

>     >

>     > Maxim.

> 

>     It's there to remind that maybe IPC should not have this dependency

>     either. All other pktios manage to do their job without this

>     conversion. So, I'm not expecting anyone adding this call outside of

>     packet.c, but instead ipc.c to remove the call after which we can

>     move the function back into packet.c.

> 

> 

> If ipc can operate without that conversion then it can be converted now,

> but my understanding is that because it operates between ODP instances

> it's something of a special case. I'll add this topic to today's ODP call.

>  

> 

> 

>     -Petri

> 

> 


In ipc we can write something like:

odp_packet_t pkt;
void *mbase = pktio_entry->s.ipc.pool_mdata_base;

pkt = (odp_packet_t)((uint8_t *)mbase + offsets[i]);


But comment we need to remove due to other platforms can inherit packet
include with symlink and may not have ipc pktio. For them that comment
will be useless.

Maxim.
Savolainen, Petri (Nokia - FI/Espoo) Feb. 28, 2017, 1:22 p.m. UTC | #7
From: Bill Fischofer [mailto:bill.fischofer@linaro.org] 

Sent: Tuesday, February 28, 2017 2:58 PM
To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com>
Cc: Maxim Uvarov <maxim.uvarov@linaro.org>; lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCH 1/2] linux-gen: ipc: use hdr to handle conversion function

Your series also includes TM (good catch) so it seems this need extends beyond just IPC. 


TM does not do pkt_hdr->pkt_hdl conversion, but buf_hdl -> pkt_hdl conversion.

-	odp_packet_t pkt = (odp_packet_t)buf_hdr->handle.handle;
+	odp_packet_t pkt = _odp_packet_from_buffer(buf_hdr->handle.handle);


Maybe IPC should not record pkt header pointers into the ring, but packet handles instead.

-Petri
Maxim Uvarov Feb. 28, 2017, 1:49 p.m. UTC | #8
On 02/28/17 16:22, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> 

> 

> From: Bill Fischofer [mailto:bill.fischofer@linaro.org] 

> Sent: Tuesday, February 28, 2017 2:58 PM

> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com>

> Cc: Maxim Uvarov <maxim.uvarov@linaro.org>; lng-odp@lists.linaro.org

> Subject: Re: [lng-odp] [PATCH 1/2] linux-gen: ipc: use hdr to handle conversion function

> 

> Your series also includes TM (good catch) so it seems this need extends beyond just IPC. 

> 

> 

> TM does not do pkt_hdr->pkt_hdl conversion, but buf_hdl -> pkt_hdl conversion.

> 

> -	odp_packet_t pkt = (odp_packet_t)buf_hdr->handle.handle;

> +	odp_packet_t pkt = _odp_packet_from_buffer(buf_hdr->handle.handle);

> 

> 

> Maybe IPC should not record pkt header pointers into the ring, but packet handles instead.

> 

> -Petri

> 

> 


We can do for now absolutely the same for IPC. That works:


                        void *mbase = pktio_entry->s.ipc.pool_mdata_base;

                        phdr = (void *)((uint8_t *)mbase + offsets[i]);
-                       pkt = (odp_packet_t)phdr->buf_hdr.handle.handle;
+                       pkt =
_odp_packet_from_buffer(phdr->buf_hdr.handle.handle);
                        odp_packet_free(pkt);
Maxim Uvarov Feb. 28, 2017, 1:54 p.m. UTC | #9
On 02/28/17 16:49, Maxim Uvarov wrote:
> On 02/28/17 16:22, Savolainen, Petri (Nokia - FI/Espoo) wrote:

>>

>>

>> From: Bill Fischofer [mailto:bill.fischofer@linaro.org] 

>> Sent: Tuesday, February 28, 2017 2:58 PM

>> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com>

>> Cc: Maxim Uvarov <maxim.uvarov@linaro.org>; lng-odp@lists.linaro.org

>> Subject: Re: [lng-odp] [PATCH 1/2] linux-gen: ipc: use hdr to handle conversion function

>>

>> Your series also includes TM (good catch) so it seems this need extends beyond just IPC. 

>>

>>

>> TM does not do pkt_hdr->pkt_hdl conversion, but buf_hdl -> pkt_hdl conversion.

>>

>> -	odp_packet_t pkt = (odp_packet_t)buf_hdr->handle.handle;

>> +	odp_packet_t pkt = _odp_packet_from_buffer(buf_hdr->handle.handle);

>>

>>

>> Maybe IPC should not record pkt header pointers into the ring, but packet handles instead.

>>

>> -Petri

>>



One of first version worked with handles.

But after that it was needed to recalculate handle to block_offset for
each packet. And multiply was match slower then base_addr + offset.


	handle.handle = buf;
	index         = handle.index;
	block_offset  = index * pool->block_size;

	/* clang requires cast to uintptr_t */
	buf_hdr = (odp_buffer_hdr_t *)(uintptr_t)&pool->base_addr[block_offset];




>>

> 

> We can do for now absolutely the same for IPC. That works:

> 

> 

>                         void *mbase = pktio_entry->s.ipc.pool_mdata_base;

> 

>                         phdr = (void *)((uint8_t *)mbase + offsets[i]);

> -                       pkt = (odp_packet_t)phdr->buf_hdr.handle.handle;

> +                       pkt =

> _odp_packet_from_buffer(phdr->buf_hdr.handle.handle);

>                         odp_packet_free(pkt);

>
Maxim Uvarov March 1, 2017, 9:16 a.m. UTC | #10
Petri, can you send v2 without note or add review to my patch? This is
stopper for current odp dev release. I would like to solve that and to 1.14
before Connect.

Maxim.

On 28 February 2017 at 16:54, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:

> On 02/28/17 16:49, Maxim Uvarov wrote:

> > On 02/28/17 16:22, Savolainen, Petri (Nokia - FI/Espoo) wrote:

> >>

> >>

> >> From: Bill Fischofer [mailto:bill.fischofer@linaro.org]

> >> Sent: Tuesday, February 28, 2017 2:58 PM

> >> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-

> labs.com>

> >> Cc: Maxim Uvarov <maxim.uvarov@linaro.org>; lng-odp@lists.linaro.org

> >> Subject: Re: [lng-odp] [PATCH 1/2] linux-gen: ipc: use hdr to handle

> conversion function

> >>

> >> Your series also includes TM (good catch) so it seems this need extends

> beyond just IPC.

> >>

> >>

> >> TM does not do pkt_hdr->pkt_hdl conversion, but buf_hdl -> pkt_hdl

> conversion.

> >>

> >> -    odp_packet_t pkt = (odp_packet_t)buf_hdr->handle.handle;

> >> +    odp_packet_t pkt = _odp_packet_from_buffer(buf_

> hdr->handle.handle);

> >>

> >>

> >> Maybe IPC should not record pkt header pointers into the ring, but

> packet handles instead.

> >>

> >> -Petri

> >>

>

>

> One of first version worked with handles.

>

> But after that it was needed to recalculate handle to block_offset for

> each packet. And multiply was match slower then base_addr + offset.

>

>

>         handle.handle = buf;

>         index         = handle.index;

>         block_offset  = index * pool->block_size;

>

>         /* clang requires cast to uintptr_t */

>         buf_hdr = (odp_buffer_hdr_t *)(uintptr_t)&pool->base_addr[

> block_offset];

>

>

>

>

> >>

> >

> > We can do for now absolutely the same for IPC. That works:

> >

> >

> >                         void *mbase = pktio_entry->s.ipc.pool_mdata_

> base;

> >

> >                         phdr = (void *)((uint8_t *)mbase + offsets[i]);

> > -                       pkt = (odp_packet_t)phdr->buf_hdr.handle.handle;

> > +                       pkt =

> > _odp_packet_from_buffer(phdr->buf_hdr.handle.handle);

> >                         odp_packet_free(pkt);

> >

>

>
diff mbox series

Patch

diff --git a/platform/linux-generic/include/odp_packet_internal.h b/platform/linux-generic/include/odp_packet_internal.h
index 4f844d1..7a394dd 100644
--- a/platform/linux-generic/include/odp_packet_internal.h
+++ b/platform/linux-generic/include/odp_packet_internal.h
@@ -163,6 +163,12 @@  static inline odp_packet_hdr_t *odp_packet_hdr(odp_packet_t pkt)
 	return (odp_packet_hdr_t *)(uintptr_t)pkt;
 }
 
+/* Only one using this outside of packet.c is ipc.c */
+static inline odp_packet_t packet_handle(odp_packet_hdr_t *pkt_hdr)
+{
+	return (odp_packet_t)pkt_hdr;
+}
+
 static inline void copy_packet_parser_metadata(odp_packet_hdr_t *src_hdr,
 					       odp_packet_hdr_t *dst_hdr)
 {
diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-generic/odp_packet.c
index c21f635..3019418 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -48,11 +48,6 @@  static inline odp_packet_hdr_t *packet_hdr(odp_packet_t pkt)
 	return (odp_packet_hdr_t *)(uintptr_t)pkt;
 }
 
-static inline odp_packet_t packet_handle(odp_packet_hdr_t *pkt_hdr)
-{
-	return (odp_packet_t)pkt_hdr;
-}
-
 static inline odp_buffer_t buffer_handle(odp_packet_hdr_t *pkt_hdr)
 {
 	return pkt_hdr->buf_hdr.handle.handle;
diff --git a/platform/linux-generic/pktio/ipc.c b/platform/linux-generic/pktio/ipc.c
index 377f20e..06175e5 100644
--- a/platform/linux-generic/pktio/ipc.c
+++ b/platform/linux-generic/pktio/ipc.c
@@ -409,7 +409,7 @@  static void _ipc_free_ring_packets(pktio_entry_t *pktio_entry, _ring_t *r)
 			void *mbase = pktio_entry->s.ipc.pool_mdata_base;
 
 			phdr = (void *)((uint8_t *)mbase + offsets[i]);
-			pkt = (odp_packet_t)phdr->buf_hdr.handle.handle;
+			pkt = packet_handle(phdr);
 			odp_packet_free(pkt);
 		}
 	}