diff mbox

[2/2] Rename packet start function

Message ID 1412331478-30433-2-git-send-email-petri.savolainen@linaro.org
State Accepted
Commit 7df90a6f64f620daf653c0462a9ddee9ef6f4935
Headers show

Commit Message

Petri Savolainen Oct. 3, 2014, 10:17 a.m. UTC
- Unify function naming between packets and segments: xxx_addr(), xxx_data()
- Rename odp_packet_start() to odp_packet_data()

Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>
---
 example/ipsec/odp_ipsec_stream.c                |  4 ++--
 platform/linux-generic/include/api/odp_packet.h | 32 +++++++++++--------------
 platform/linux-generic/odp_packet.c             |  8 +++----
 3 files changed, 20 insertions(+), 24 deletions(-)

Comments

Gilad Ben-Yossef Oct. 6, 2014, 6:08 a.m. UTC | #1
Hi Petri,
Thanks for the rename. 
While we're touching the packet/buffer data access APIs - I'm a little puzzled on how are these APIs supposed to work if the packet is segmented?
Let's say odp_packet_data() handed you a pointer to packet data of a segmented packet - how do you know how much data is there that you can access?
Is the expectation that every call to odp_packet_data() will copy all segmented packet data to make them contiguous? This sound a bit extreme to me as you might end up memcpy 9k of data just to let someone peek at the Ethernet header...
Thanks,
Gilad

Gilad Ben-Yossef
Software Architect
EZchip Technologies Ltd.
37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel
Tel: +972-4-959-6666 ext. 576, Fax: +972-8-681-1483 
Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388
Email: giladb@ezchip.com, Web: http://www.ezchip.com

"Ethernet always wins."
        — Andy Bechtolsheim

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

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

> bounces@lists.linaro.org] On Behalf Of Petri Savolainen

> Sent: Friday, October 03, 2014 1:18 PM

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

> Subject: [lng-odp] [PATCH 2/2] Rename packet start function

> 

> - Unify function naming between packets and segments: xxx_addr(),

> xxx_data()

> - Rename odp_packet_start() to odp_packet_data()

> 

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

> ---

>  example/ipsec/odp_ipsec_stream.c                |  4 ++--

>  platform/linux-generic/include/api/odp_packet.h | 32 +++++++++++--------

> ------

>  platform/linux-generic/odp_packet.c             |  8 +++----

>  3 files changed, 20 insertions(+), 24 deletions(-)

> 

> diff --git a/example/ipsec/odp_ipsec_stream.c

> b/example/ipsec/odp_ipsec_stream.c

> index de8305a..fba425c 100644

> --- a/example/ipsec/odp_ipsec_stream.c

> +++ b/example/ipsec/odp_ipsec_stream.c

> @@ -193,8 +193,8 @@ odp_packet_t create_ipv4_packet(stream_db_entry_t

> *stream,

>  		return ODP_PACKET_INVALID;

>  	pkt = odp_packet_from_buffer(bfr);

>  	odp_packet_init(pkt);

> -	base = odp_packet_start(pkt);

> -	data = odp_packet_start(pkt);

> +	base = odp_packet_data(pkt);

> +	data = odp_packet_data(pkt);

> 

>  	/* Ethernet */

>  	odp_packet_set_inflag_eth(pkt, 1);

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

> b/platform/linux-generic/include/api/odp_packet.h

> index 83f345a..316279b 100644

> --- a/platform/linux-generic/include/api/odp_packet.h

> +++ b/platform/linux-generic/include/api/odp_packet.h

> @@ -116,41 +116,37 @@ void odp_packet_set_ctx(odp_packet_t buf, const

> void *ctx);

>  void *odp_packet_get_ctx(odp_packet_t buf);

> 

>  /**

> - * Get address to the start of the packet buffer

> + * Packet buffer start address

> + *

> + * Returns a pointer to the start of the packet buffer. The address is

> not

> + * necessarily the same as packet data address. E.g. on a received

> Ethernet

> + * frame, the protocol header may start 2 or 6 bytes within the buffer

> to

> + * ensure 32 or 64-bit alignment of the IP header.

>   *

> - * The address of the packet buffer is not necessarily the same as the

> start

> - * address of the received frame, e.g. an eth frame may be offset by 2

> or 6

> - * bytes to ensure 32 or 64-bit alignment of the IP header.

>   * Use odp_packet_l2(pkt) to get the start address of a received valid

> frame

> - * or odp_packet_start(pkt) to get the start address even if no valid L2

> header

> - * could be found.

> + * or odp_packet_data(pkt) to get the current packet data address.

>   *

>   * @param pkt  Packet handle

>   *

>   * @return  Pointer to the start of the packet buffer

>   *

> - * @see odp_packet_l2(), odp_packet_start()

> + * @see odp_packet_l2(), odp_packet_data()

>   */

>  uint8_t *odp_packet_addr(odp_packet_t pkt);

> 

>  /**

> - * Get pointer to the start of the received frame

> - *

> - * The address of the packet buffer is not necessarily the same as the

> start

> - * address of the received frame, e.g. an eth frame may be offset by 2

> or 6

> - * bytes to ensure 32 or 64-bit alignment of the IP header.

> - * Use odp_packet_l2(pkt) to get the start address of a received valid

> eth frame

> + * Packet data address

>   *

> - * odp_packet_start() will always return a pointer to the start of the

> frame,

> - * even if the frame is unrecognized and no valid L2 header could be

> found.

> + * Returns the current packet data address. When a packet is received

> from

> + * packet input, the data address points to the first byte of the

> packet.

>   *

>   * @param pkt  Packet handle

>   *

> - * @return  Pointer to the start of the received frame

> + * @return  Pointer to the packet data

>   *

>   * @see odp_packet_l2(), odp_packet_addr()

>   */

> -uint8_t *odp_packet_start(odp_packet_t pkt);

> +uint8_t *odp_packet_data(odp_packet_t pkt);

> 

>  /**

>   * Get pointer to the start of the L2 frame

> @@ -162,7 +158,7 @@ uint8_t *odp_packet_start(odp_packet_t pkt);

>   *

>   * @return  Pointer to L2 header or NULL if not found

>   *

> - * @see odp_packet_addr(), odp_packet_start()

> + * @see odp_packet_addr(), odp_packet_data()

>   */

>  uint8_t *odp_packet_l2(odp_packet_t pkt);

> 

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

> generic/odp_packet.c

> index 6cd6183..435bc8b 100644

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

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

> @@ -61,7 +61,7 @@ uint8_t *odp_packet_addr(odp_packet_t pkt)

>  	return odp_buffer_addr(odp_buffer_from_packet(pkt));

>  }

> 

> -uint8_t *odp_packet_start(odp_packet_t pkt)

> +uint8_t *odp_packet_data(odp_packet_t pkt)

>  {

>  	return odp_packet_addr(pkt) + odp_packet_hdr(pkt)->frame_offset;

>  }

> @@ -183,7 +183,7 @@ void odp_packet_parse(odp_packet_t pkt, size_t len,

> size_t frame_offset)

>  	pkt_hdr->input_flags.l2 = 1;

>  	pkt_hdr->l2_offset = frame_offset;

> 

> -	eth = (odph_ethhdr_t *)odp_packet_start(pkt);

> +	eth = (odph_ethhdr_t *)odp_packet_data(pkt);

>  	ethtype = odp_be_to_cpu_16(eth->type);

>  	vlan = (odph_vlanhdr_t *)&eth->type;

> 

> @@ -372,8 +372,8 @@ int odp_packet_copy(odp_packet_t pkt_dst,

> odp_packet_t pkt_src)

>  	memcpy(start_dst, start_src, len);

> 

>  	/* Copy frame payload */

> -	start_dst = (uint8_t *)odp_packet_start(pkt_dst);

> -	start_src = (uint8_t *)odp_packet_start(pkt_src);

> +	start_dst = (uint8_t *)odp_packet_data(pkt_dst);

> +	start_src = (uint8_t *)odp_packet_data(pkt_src);

>  	len = pkt_hdr_src->frame_len;

>  	memcpy(start_dst, start_src, len);

> 

> --

> 2.1.1

> 

> 

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> http://lists.linaro.org/mailman/listinfo/lng-odp
Rosenboim, Leonid Oct. 6, 2014, 6:25 a.m. UTC | #2
I agree with Gilad.

There are two ways to deal with this:

One method is to NEVER return a pointer into the packet buffer, and instead provide function that  "push" and "pop" headers, which copy the relevant headers to/from caller-provided storage.

The other method is to assign a Unix read()-like character to the functions that retrieve pointers into the packet buffer, and have then return both the pointer as well as the size of data available at that address contiguously. Of course either the pointer or the length would deed to be passed via a by-reference argument.

Let us however not start a debate about which of these two methods is superior, PLEASE!
Just chose one and go with it.

- Leo
Jerin Jacob Oct. 6, 2014, 6:42 a.m. UTC | #3
On Mon, Oct 06, 2014 at 06:08:07AM +0000, Gilad Ben Yossef wrote:
> Hi Petri,
> Thanks for the rename. 
> While we're touching the packet/buffer data access APIs - I'm a little puzzled on how are these APIs supposed to work if the packet is segmented?
> Let's say odp_packet_data() handed you a pointer to packet data of a segmented packet - how do you know how much data is there that you can access?

I guess, In case of segmented packet, We need to use "odp_buffer_segment_next" equivalent in odp_packet domain.
example code snippet  taken from odp buffer management documentation.

odp_buffer_t buf;
odp_buffer_segment_t seg = odp_buffer_segment_next(buf,ODP_SEGMENT_START);
size_t seglen;

while (seg != ODP_SEGMENT_NULL) {
        char *segptr = odp_buffer_segment_map(buf,seg,&seglen);
        ...do something with this segment
        odp_buffer_segment_unmap(seg);
        seg = odp_buffer_segment_next(buf,seg);
}


> Is the expectation that every call to odp_packet_data() will copy all segmented packet data to make them contiguous? This sound a bit extreme to me as you might end up memcpy 9k of data just to let someone peek at the Ethernet header...
> Thanks,
> Gilad
> 
> Gilad Ben-Yossef
> Software Architect
> EZchip Technologies Ltd.
> 37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel
> Tel: +972-4-959-6666 ext. 576, Fax: +972-8-681-1483 
> Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388
> Email: giladb@ezchip.com, Web: http://www.ezchip.com
> 
> "Ethernet always wins."
>         — Andy Bechtolsheim
> 
> > -----Original Message-----
> > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> > bounces@lists.linaro.org] On Behalf Of Petri Savolainen
> > Sent: Friday, October 03, 2014 1:18 PM
> > To: lng-odp@lists.linaro.org
> > Subject: [lng-odp] [PATCH 2/2] Rename packet start function
> > 
> > - Unify function naming between packets and segments: xxx_addr(),
> > xxx_data()
> > - Rename odp_packet_start() to odp_packet_data()
> > 
> > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>
> > ---
> >  example/ipsec/odp_ipsec_stream.c                |  4 ++--
> >  platform/linux-generic/include/api/odp_packet.h | 32 +++++++++++--------
> > ------
> >  platform/linux-generic/odp_packet.c             |  8 +++----
> >  3 files changed, 20 insertions(+), 24 deletions(-)
> > 
> > diff --git a/example/ipsec/odp_ipsec_stream.c
> > b/example/ipsec/odp_ipsec_stream.c
> > index de8305a..fba425c 100644
> > --- a/example/ipsec/odp_ipsec_stream.c
> > +++ b/example/ipsec/odp_ipsec_stream.c
> > @@ -193,8 +193,8 @@ odp_packet_t create_ipv4_packet(stream_db_entry_t
> > *stream,
> >  		return ODP_PACKET_INVALID;
> >  	pkt = odp_packet_from_buffer(bfr);
> >  	odp_packet_init(pkt);
> > -	base = odp_packet_start(pkt);
> > -	data = odp_packet_start(pkt);
> > +	base = odp_packet_data(pkt);
> > +	data = odp_packet_data(pkt);
> > 
> >  	/* Ethernet */
> >  	odp_packet_set_inflag_eth(pkt, 1);
> > diff --git a/platform/linux-generic/include/api/odp_packet.h
> > b/platform/linux-generic/include/api/odp_packet.h
> > index 83f345a..316279b 100644
> > --- a/platform/linux-generic/include/api/odp_packet.h
> > +++ b/platform/linux-generic/include/api/odp_packet.h
> > @@ -116,41 +116,37 @@ void odp_packet_set_ctx(odp_packet_t buf, const
> > void *ctx);
> >  void *odp_packet_get_ctx(odp_packet_t buf);
> > 
> >  /**
> > - * Get address to the start of the packet buffer
> > + * Packet buffer start address
> > + *
> > + * Returns a pointer to the start of the packet buffer. The address is
> > not
> > + * necessarily the same as packet data address. E.g. on a received
> > Ethernet
> > + * frame, the protocol header may start 2 or 6 bytes within the buffer
> > to
> > + * ensure 32 or 64-bit alignment of the IP header.
> >   *
> > - * The address of the packet buffer is not necessarily the same as the
> > start
> > - * address of the received frame, e.g. an eth frame may be offset by 2
> > or 6
> > - * bytes to ensure 32 or 64-bit alignment of the IP header.
> >   * Use odp_packet_l2(pkt) to get the start address of a received valid
> > frame
> > - * or odp_packet_start(pkt) to get the start address even if no valid L2
> > header
> > - * could be found.
> > + * or odp_packet_data(pkt) to get the current packet data address.
> >   *
> >   * @param pkt  Packet handle
> >   *
> >   * @return  Pointer to the start of the packet buffer
> >   *
> > - * @see odp_packet_l2(), odp_packet_start()
> > + * @see odp_packet_l2(), odp_packet_data()
> >   */
> >  uint8_t *odp_packet_addr(odp_packet_t pkt);
> > 
> >  /**
> > - * Get pointer to the start of the received frame
> > - *
> > - * The address of the packet buffer is not necessarily the same as the
> > start
> > - * address of the received frame, e.g. an eth frame may be offset by 2
> > or 6
> > - * bytes to ensure 32 or 64-bit alignment of the IP header.
> > - * Use odp_packet_l2(pkt) to get the start address of a received valid
> > eth frame
> > + * Packet data address
> >   *
> > - * odp_packet_start() will always return a pointer to the start of the
> > frame,
> > - * even if the frame is unrecognized and no valid L2 header could be
> > found.
> > + * Returns the current packet data address. When a packet is received
> > from
> > + * packet input, the data address points to the first byte of the
> > packet.
> >   *
> >   * @param pkt  Packet handle
> >   *
> > - * @return  Pointer to the start of the received frame
> > + * @return  Pointer to the packet data
> >   *
> >   * @see odp_packet_l2(), odp_packet_addr()
> >   */
> > -uint8_t *odp_packet_start(odp_packet_t pkt);
> > +uint8_t *odp_packet_data(odp_packet_t pkt);
> > 
> >  /**
> >   * Get pointer to the start of the L2 frame
> > @@ -162,7 +158,7 @@ uint8_t *odp_packet_start(odp_packet_t pkt);
> >   *
> >   * @return  Pointer to L2 header or NULL if not found
> >   *
> > - * @see odp_packet_addr(), odp_packet_start()
> > + * @see odp_packet_addr(), odp_packet_data()
> >   */
> >  uint8_t *odp_packet_l2(odp_packet_t pkt);
> > 
> > diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-
> > generic/odp_packet.c
> > index 6cd6183..435bc8b 100644
> > --- a/platform/linux-generic/odp_packet.c
> > +++ b/platform/linux-generic/odp_packet.c
> > @@ -61,7 +61,7 @@ uint8_t *odp_packet_addr(odp_packet_t pkt)
> >  	return odp_buffer_addr(odp_buffer_from_packet(pkt));
> >  }
> > 
> > -uint8_t *odp_packet_start(odp_packet_t pkt)
> > +uint8_t *odp_packet_data(odp_packet_t pkt)
> >  {
> >  	return odp_packet_addr(pkt) + odp_packet_hdr(pkt)->frame_offset;
> >  }
> > @@ -183,7 +183,7 @@ void odp_packet_parse(odp_packet_t pkt, size_t len,
> > size_t frame_offset)
> >  	pkt_hdr->input_flags.l2 = 1;
> >  	pkt_hdr->l2_offset = frame_offset;
> > 
> > -	eth = (odph_ethhdr_t *)odp_packet_start(pkt);
> > +	eth = (odph_ethhdr_t *)odp_packet_data(pkt);
> >  	ethtype = odp_be_to_cpu_16(eth->type);
> >  	vlan = (odph_vlanhdr_t *)&eth->type;
> > 
> > @@ -372,8 +372,8 @@ int odp_packet_copy(odp_packet_t pkt_dst,
> > odp_packet_t pkt_src)
> >  	memcpy(start_dst, start_src, len);
> > 
> >  	/* Copy frame payload */
> > -	start_dst = (uint8_t *)odp_packet_start(pkt_dst);
> > -	start_src = (uint8_t *)odp_packet_start(pkt_src);
> > +	start_dst = (uint8_t *)odp_packet_data(pkt_dst);
> > +	start_src = (uint8_t *)odp_packet_data(pkt_src);
> >  	len = pkt_hdr_src->frame_len;
> >  	memcpy(start_dst, start_src, len);
> > 
> > --
> > 2.1.1
> > 
> > 
> > _______________________________________________
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/lng-odp
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
Gilad Ben-Yossef Oct. 6, 2014, 7:43 a.m. UTC | #4
Thanks Leo,
I think the notion that packet buffers are trivially globally addressable is a limiting design choice even if the packet is not segmented.
Because of the project status, maybe the best thing is just to leave the APIs as is for ODP 1.0 and document they the API is only relevant if the packet is not segmented (and otherwise returns an error) - but I think it's fair to say we should revisit these APIs quickly after 1.0.
Consider NFV for example - a vSwitch gets frames for VMs running ODP. If you assume packet buffers are trivially globally addressable, either all VMs sees the memory where packet buffers are allocated from (which is bad for isolation and security) or the vSwitch will have to perform a full packet buffer copy to each VM memory space.
With a push/pop like notion, the vSwitch can gives the ODP programs in the VMs buffer descriptors and the pop/push operation copies or maps just the part the specific ODP program in the VM are interested in. It can be the difference between copying 24 bytes of Ethernet header in and out or a full 9k.
Same thing with IPC between different ODP  program.
Gilad Ben-Yossef
Software Architect
EZchip Technologies Ltd.
37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel
Tel: +972-4-959-6666 ext. 576, Fax: +972-8-681-1483 
Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388
Email: giladb@ezchip.com, Web: http://www.ezchip.com

"Ethernet always wins."
        — Andy Bechtolsheim


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

> From: Rosenboim, Leonid [mailto:Leonid.Rosenboim@caviumnetworks.com]

> Sent: Monday, October 06, 2014 9:26 AM

> To: Gilad Ben Yossef; Petri Savolainen; lng-odp@lists.linaro.org

> Subject: Re: [lng-odp] [PATCH 2/2] Rename packet start function

> 

> I agree with Gilad.

> 

> There are two ways to deal with this:

> 

> One method is to NEVER return a pointer into the packet buffer, and

> instead provide function that  "push" and "pop" headers, which copy the

> relevant headers to/from caller-provided storage.

> 

> The other method is to assign a Unix read()-like character to the

> functions that retrieve pointers into the packet buffer, and have then

> return both the pointer as well as the size of data available at that

> address contiguously. Of course either the pointer or the length would

> deed to be passed via a by-reference argument.

> 

> Let us however not start a debate about which of these two methods is

> superior, PLEASE!

> Just chose one and go with it.

> 

> - Leo

> ________________________________________

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

> on behalf of Gilad Ben Yossef <giladb@ezchip.com>

> Sent: Sunday, October 05, 2014 11:08 PM

> To: Petri Savolainen; lng-odp@lists.linaro.org

> Subject: Re: [lng-odp] [PATCH 2/2] Rename packet start function

> 

> Hi Petri,

> Thanks for the rename.

> While we're touching the packet/buffer data access APIs - I'm a little

> puzzled on how are these APIs supposed to work if the packet is

> segmented?

> Let's say odp_packet_data() handed you a pointer to packet data of a

> segmented packet - how do you know how much data is there that you can

> access?

> Is the expectation that every call to odp_packet_data() will copy all

> segmented packet data to make them contiguous? This sound a bit extreme

> to me as you might end up memcpy 9k of data just to let someone peek at

> the Ethernet header...

> Thanks,

> Gilad

> 

> Gilad Ben-Yossef

> Software Architect

> EZchip Technologies Ltd.

> 37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel

> Tel: +972-4-959-6666 ext. 576, Fax: +972-8-681-1483

> Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388

> Email: giladb@ezchip.com, Web: http://www.ezchip.com

> 

> "Ethernet always wins."

>         — Andy Bechtolsheim

> 

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

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

> > bounces@lists.linaro.org] On Behalf Of Petri Savolainen

> > Sent: Friday, October 03, 2014 1:18 PM

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

> > Subject: [lng-odp] [PATCH 2/2] Rename packet start function

> >

> > - Unify function naming between packets and segments: xxx_addr(),

> > xxx_data()

> > - Rename odp_packet_start() to odp_packet_data()

> >

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

> > ---

> >  example/ipsec/odp_ipsec_stream.c                |  4 ++--

> >  platform/linux-generic/include/api/odp_packet.h | 32 +++++++++++------

> --

> > ------

> >  platform/linux-generic/odp_packet.c             |  8 +++----

> >  3 files changed, 20 insertions(+), 24 deletions(-)

> >

> > diff --git a/example/ipsec/odp_ipsec_stream.c

> > b/example/ipsec/odp_ipsec_stream.c

> > index de8305a..fba425c 100644

> > --- a/example/ipsec/odp_ipsec_stream.c

> > +++ b/example/ipsec/odp_ipsec_stream.c

> > @@ -193,8 +193,8 @@ odp_packet_t create_ipv4_packet(stream_db_entry_t

> > *stream,

> >               return ODP_PACKET_INVALID;

> >       pkt = odp_packet_from_buffer(bfr);

> >       odp_packet_init(pkt);

> > -     base = odp_packet_start(pkt);

> > -     data = odp_packet_start(pkt);

> > +     base = odp_packet_data(pkt);

> > +     data = odp_packet_data(pkt);

> >

> >       /* Ethernet */

> >       odp_packet_set_inflag_eth(pkt, 1);

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

> > b/platform/linux-generic/include/api/odp_packet.h

> > index 83f345a..316279b 100644

> > --- a/platform/linux-generic/include/api/odp_packet.h

> > +++ b/platform/linux-generic/include/api/odp_packet.h

> > @@ -116,41 +116,37 @@ void odp_packet_set_ctx(odp_packet_t buf, const

> > void *ctx);

> >  void *odp_packet_get_ctx(odp_packet_t buf);

> >

> >  /**

> > - * Get address to the start of the packet buffer

> > + * Packet buffer start address

> > + *

> > + * Returns a pointer to the start of the packet buffer. The address is

> > not

> > + * necessarily the same as packet data address. E.g. on a received

> > Ethernet

> > + * frame, the protocol header may start 2 or 6 bytes within the buffer

> > to

> > + * ensure 32 or 64-bit alignment of the IP header.

> >   *

> > - * The address of the packet buffer is not necessarily the same as the

> > start

> > - * address of the received frame, e.g. an eth frame may be offset by 2

> > or 6

> > - * bytes to ensure 32 or 64-bit alignment of the IP header.

> >   * Use odp_packet_l2(pkt) to get the start address of a received valid

> > frame

> > - * or odp_packet_start(pkt) to get the start address even if no valid

> L2

> > header

> > - * could be found.

> > + * or odp_packet_data(pkt) to get the current packet data address.

> >   *

> >   * @param pkt  Packet handle

> >   *

> >   * @return  Pointer to the start of the packet buffer

> >   *

> > - * @see odp_packet_l2(), odp_packet_start()

> > + * @see odp_packet_l2(), odp_packet_data()

> >   */

> >  uint8_t *odp_packet_addr(odp_packet_t pkt);

> >

> >  /**

> > - * Get pointer to the start of the received frame

> > - *

> > - * The address of the packet buffer is not necessarily the same as the

> > start

> > - * address of the received frame, e.g. an eth frame may be offset by 2

> > or 6

> > - * bytes to ensure 32 or 64-bit alignment of the IP header.

> > - * Use odp_packet_l2(pkt) to get the start address of a received valid

> > eth frame

> > + * Packet data address

> >   *

> > - * odp_packet_start() will always return a pointer to the start of the

> > frame,

> > - * even if the frame is unrecognized and no valid L2 header could be

> > found.

> > + * Returns the current packet data address. When a packet is received

> > from

> > + * packet input, the data address points to the first byte of the

> > packet.

> >   *

> >   * @param pkt  Packet handle

> >   *

> > - * @return  Pointer to the start of the received frame

> > + * @return  Pointer to the packet data

> >   *

> >   * @see odp_packet_l2(), odp_packet_addr()

> >   */

> > -uint8_t *odp_packet_start(odp_packet_t pkt);

> > +uint8_t *odp_packet_data(odp_packet_t pkt);

> >

> >  /**

> >   * Get pointer to the start of the L2 frame

> > @@ -162,7 +158,7 @@ uint8_t *odp_packet_start(odp_packet_t pkt);

> >   *

> >   * @return  Pointer to L2 header or NULL if not found

> >   *

> > - * @see odp_packet_addr(), odp_packet_start()

> > + * @see odp_packet_addr(), odp_packet_data()

> >   */

> >  uint8_t *odp_packet_l2(odp_packet_t pkt);

> >

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

> > generic/odp_packet.c

> > index 6cd6183..435bc8b 100644

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

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

> > @@ -61,7 +61,7 @@ uint8_t *odp_packet_addr(odp_packet_t pkt)

> >       return odp_buffer_addr(odp_buffer_from_packet(pkt));

> >  }

> >

> > -uint8_t *odp_packet_start(odp_packet_t pkt)

> > +uint8_t *odp_packet_data(odp_packet_t pkt)

> >  {

> >       return odp_packet_addr(pkt) + odp_packet_hdr(pkt)->frame_offset;

> >  }

> > @@ -183,7 +183,7 @@ void odp_packet_parse(odp_packet_t pkt, size_t len,

> > size_t frame_offset)

> >       pkt_hdr->input_flags.l2 = 1;

> >       pkt_hdr->l2_offset = frame_offset;

> >

> > -     eth = (odph_ethhdr_t *)odp_packet_start(pkt);

> > +     eth = (odph_ethhdr_t *)odp_packet_data(pkt);

> >       ethtype = odp_be_to_cpu_16(eth->type);

> >       vlan = (odph_vlanhdr_t *)&eth->type;

> >

> > @@ -372,8 +372,8 @@ int odp_packet_copy(odp_packet_t pkt_dst,

> > odp_packet_t pkt_src)

> >       memcpy(start_dst, start_src, len);

> >

> >       /* Copy frame payload */

> > -     start_dst = (uint8_t *)odp_packet_start(pkt_dst);

> > -     start_src = (uint8_t *)odp_packet_start(pkt_src);

> > +     start_dst = (uint8_t *)odp_packet_data(pkt_dst);

> > +     start_src = (uint8_t *)odp_packet_data(pkt_src);

> >       len = pkt_hdr_src->frame_len;

> >       memcpy(start_dst, start_src, len);

> >

> > --

> > 2.1.1

> >

> >

> > _______________________________________________

> > lng-odp mailing list

> > lng-odp@lists.linaro.org

> > http://lists.linaro.org/mailman/listinfo/lng-odp

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> http://lists.linaro.org/mailman/listinfo/lng-odp
Gilad Ben-Yossef Oct. 6, 2014, 7:48 a.m. UTC | #5
Yeah, the problem is that the application writer needs to check if the packet she got is segmented and she can only use all these APIs if it doesn't. And what happens if you do use the API with a segmented packet? Do you get a pointer to whatever is in the first segment and it's up to the application writer to make sure she doesn't go across segment boundaries?
That can work, and is probably the simplest for 1.0 given the project goals for an end of year release, but we need to document it I guess.
Gilad 

Gilad Ben-Yossef
Software Architect
EZchip Technologies Ltd.
37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel
Tel: +972-4-959-6666 ext. 576, Fax: +972-8-681-1483 
Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388
Email: giladb@ezchip.com, Web: http://www.ezchip.com

"Ethernet always wins."
        — Andy Bechtolsheim


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

> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]

> Sent: Monday, October 06, 2014 9:42 AM

> To: Gilad Ben Yossef

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

> Subject: Re: [lng-odp] [PATCH 2/2] Rename packet start function

> 

> On Mon, Oct 06, 2014 at 06:08:07AM +0000, Gilad Ben Yossef wrote:

> > Hi Petri,

> > Thanks for the rename.

> > While we're touching the packet/buffer data access APIs - I'm a little

> puzzled on how are these APIs supposed to work if the packet is

> segmented?

> > Let's say odp_packet_data() handed you a pointer to packet data of a

> segmented packet - how do you know how much data is there that you can

> access?

> 

> I guess, In case of segmented packet, We need to use

> "odp_buffer_segment_next" equivalent in odp_packet domain.

> example code snippet  taken from odp buffer management documentation.

> 

> odp_buffer_t buf;

> odp_buffer_segment_t seg =

> odp_buffer_segment_next(buf,ODP_SEGMENT_START);

> size_t seglen;

> 

> while (seg != ODP_SEGMENT_NULL) {

>         char *segptr = odp_buffer_segment_map(buf,seg,&seglen);

>         ...do something with this segment

>         odp_buffer_segment_unmap(seg);

>         seg = odp_buffer_segment_next(buf,seg);

> }

> 

> 

> > Is the expectation that every call to odp_packet_data() will copy all

> segmented packet data to make them contiguous? This sound a bit extreme

> to me as you might end up memcpy 9k of data just to let someone peek at

> the Ethernet header...

> > Thanks,

> > Gilad

> >

> > Gilad Ben-Yossef

> > Software Architect

> > EZchip Technologies Ltd.

> > 37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel

> > Tel: +972-4-959-6666 ext. 576, Fax: +972-8-681-1483

> > Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388

> > Email: giladb@ezchip.com, Web: http://www.ezchip.com

> >

> > "Ethernet always wins."

> >         — Andy Bechtolsheim

> >

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

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

> > > bounces@lists.linaro.org] On Behalf Of Petri Savolainen

> > > Sent: Friday, October 03, 2014 1:18 PM

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

> > > Subject: [lng-odp] [PATCH 2/2] Rename packet start function

> > >

> > > - Unify function naming between packets and segments: xxx_addr(),

> > > xxx_data()

> > > - Rename odp_packet_start() to odp_packet_data()

> > >

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

> > > ---

> > >  example/ipsec/odp_ipsec_stream.c                |  4 ++--

> > >  platform/linux-generic/include/api/odp_packet.h | 32 +++++++++++----

> ----

> > > ------

> > >  platform/linux-generic/odp_packet.c             |  8 +++----

> > >  3 files changed, 20 insertions(+), 24 deletions(-)

> > >

> > > diff --git a/example/ipsec/odp_ipsec_stream.c

> > > b/example/ipsec/odp_ipsec_stream.c

> > > index de8305a..fba425c 100644

> > > --- a/example/ipsec/odp_ipsec_stream.c

> > > +++ b/example/ipsec/odp_ipsec_stream.c

> > > @@ -193,8 +193,8 @@ odp_packet_t create_ipv4_packet(stream_db_entry_t

> > > *stream,

> > >  		return ODP_PACKET_INVALID;

> > >  	pkt = odp_packet_from_buffer(bfr);

> > >  	odp_packet_init(pkt);

> > > -	base = odp_packet_start(pkt);

> > > -	data = odp_packet_start(pkt);

> > > +	base = odp_packet_data(pkt);

> > > +	data = odp_packet_data(pkt);

> > >

> > >  	/* Ethernet */

> > >  	odp_packet_set_inflag_eth(pkt, 1);

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

> > > b/platform/linux-generic/include/api/odp_packet.h

> > > index 83f345a..316279b 100644

> > > --- a/platform/linux-generic/include/api/odp_packet.h

> > > +++ b/platform/linux-generic/include/api/odp_packet.h

> > > @@ -116,41 +116,37 @@ void odp_packet_set_ctx(odp_packet_t buf, const

> > > void *ctx);

> > >  void *odp_packet_get_ctx(odp_packet_t buf);

> > >

> > >  /**

> > > - * Get address to the start of the packet buffer

> > > + * Packet buffer start address

> > > + *

> > > + * Returns a pointer to the start of the packet buffer. The address

> is

> > > not

> > > + * necessarily the same as packet data address. E.g. on a received

> > > Ethernet

> > > + * frame, the protocol header may start 2 or 6 bytes within the

> buffer

> > > to

> > > + * ensure 32 or 64-bit alignment of the IP header.

> > >   *

> > > - * The address of the packet buffer is not necessarily the same as

> the

> > > start

> > > - * address of the received frame, e.g. an eth frame may be offset by

> 2

> > > or 6

> > > - * bytes to ensure 32 or 64-bit alignment of the IP header.

> > >   * Use odp_packet_l2(pkt) to get the start address of a received

> valid

> > > frame

> > > - * or odp_packet_start(pkt) to get the start address even if no

> valid L2

> > > header

> > > - * could be found.

> > > + * or odp_packet_data(pkt) to get the current packet data address.

> > >   *

> > >   * @param pkt  Packet handle

> > >   *

> > >   * @return  Pointer to the start of the packet buffer

> > >   *

> > > - * @see odp_packet_l2(), odp_packet_start()

> > > + * @see odp_packet_l2(), odp_packet_data()

> > >   */

> > >  uint8_t *odp_packet_addr(odp_packet_t pkt);

> > >

> > >  /**

> > > - * Get pointer to the start of the received frame

> > > - *

> > > - * The address of the packet buffer is not necessarily the same as

> the

> > > start

> > > - * address of the received frame, e.g. an eth frame may be offset by

> 2

> > > or 6

> > > - * bytes to ensure 32 or 64-bit alignment of the IP header.

> > > - * Use odp_packet_l2(pkt) to get the start address of a received

> valid

> > > eth frame

> > > + * Packet data address

> > >   *

> > > - * odp_packet_start() will always return a pointer to the start of

> the

> > > frame,

> > > - * even if the frame is unrecognized and no valid L2 header could be

> > > found.

> > > + * Returns the current packet data address. When a packet is

> received

> > > from

> > > + * packet input, the data address points to the first byte of the

> > > packet.

> > >   *

> > >   * @param pkt  Packet handle

> > >   *

> > > - * @return  Pointer to the start of the received frame

> > > + * @return  Pointer to the packet data

> > >   *

> > >   * @see odp_packet_l2(), odp_packet_addr()

> > >   */

> > > -uint8_t *odp_packet_start(odp_packet_t pkt);

> > > +uint8_t *odp_packet_data(odp_packet_t pkt);

> > >

> > >  /**

> > >   * Get pointer to the start of the L2 frame

> > > @@ -162,7 +158,7 @@ uint8_t *odp_packet_start(odp_packet_t pkt);

> > >   *

> > >   * @return  Pointer to L2 header or NULL if not found

> > >   *

> > > - * @see odp_packet_addr(), odp_packet_start()

> > > + * @see odp_packet_addr(), odp_packet_data()

> > >   */

> > >  uint8_t *odp_packet_l2(odp_packet_t pkt);

> > >

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

> > > generic/odp_packet.c

> > > index 6cd6183..435bc8b 100644

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

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

> > > @@ -61,7 +61,7 @@ uint8_t *odp_packet_addr(odp_packet_t pkt)

> > >  	return odp_buffer_addr(odp_buffer_from_packet(pkt));

> > >  }

> > >

> > > -uint8_t *odp_packet_start(odp_packet_t pkt)

> > > +uint8_t *odp_packet_data(odp_packet_t pkt)

> > >  {

> > >  	return odp_packet_addr(pkt) + odp_packet_hdr(pkt)->frame_offset;

> > >  }

> > > @@ -183,7 +183,7 @@ void odp_packet_parse(odp_packet_t pkt, size_t

> len,

> > > size_t frame_offset)

> > >  	pkt_hdr->input_flags.l2 = 1;

> > >  	pkt_hdr->l2_offset = frame_offset;

> > >

> > > -	eth = (odph_ethhdr_t *)odp_packet_start(pkt);

> > > +	eth = (odph_ethhdr_t *)odp_packet_data(pkt);

> > >  	ethtype = odp_be_to_cpu_16(eth->type);

> > >  	vlan = (odph_vlanhdr_t *)&eth->type;

> > >

> > > @@ -372,8 +372,8 @@ int odp_packet_copy(odp_packet_t pkt_dst,

> > > odp_packet_t pkt_src)

> > >  	memcpy(start_dst, start_src, len);

> > >

> > >  	/* Copy frame payload */

> > > -	start_dst = (uint8_t *)odp_packet_start(pkt_dst);

> > > -	start_src = (uint8_t *)odp_packet_start(pkt_src);

> > > +	start_dst = (uint8_t *)odp_packet_data(pkt_dst);

> > > +	start_src = (uint8_t *)odp_packet_data(pkt_src);

> > >  	len = pkt_hdr_src->frame_len;

> > >  	memcpy(start_dst, start_src, len);

> > >

> > > --

> > > 2.1.1

> > >

> > >

> > > _______________________________________________

> > > lng-odp mailing list

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

> > > http://lists.linaro.org/mailman/listinfo/lng-odp

> > _______________________________________________

> > lng-odp mailing list

> > lng-odp@lists.linaro.org

> > http://lists.linaro.org/mailman/listinfo/lng-odp
Savolainen, Petri (NSN - FI/Espoo) Oct. 6, 2014, 8:05 a.m. UTC | #6
Hi Gilad,


odp_packet_is_segmented() will tell the user if a packet is segmented. If it's segmented, there are a series of packet segment functions (odp_packet_seg_xxx) to apply on individual segments. Packet level functions (odp_packet_xxx) is used when packet is not segmented. I'm renaming/modifying the packet level size/len functions next. The naming logic is the following:

xxx_addr     = buffer start address
xxx_size     = buffer size
xxx_data     = pointer to the data within the buffer
xxx_data_len = data length

data and data_len cannot be set directly, but can be moved with specific functions (see odp_packet_seg_push/pull in current odp_packet.h).

headroom len = data - addr
tailroom len = size - headroom - data_len


-Petri

 

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

> From: ext Gilad Ben Yossef [mailto:giladb@ezchip.com]

> Sent: Monday, October 06, 2014 9:08 AM

> To: Petri Savolainen; lng-odp@lists.linaro.org

> Subject: RE: [lng-odp] [PATCH 2/2] Rename packet start function

> 

> Hi Petri,

> Thanks for the rename.

> While we're touching the packet/buffer data access APIs - I'm a little

> puzzled on how are these APIs supposed to work if the packet is segmented?

> Let's say odp_packet_data() handed you a pointer to packet data of a

> segmented packet - how do you know how much data is there that you can

> access?

> Is the expectation that every call to odp_packet_data() will copy all

> segmented packet data to make them contiguous? This sound a bit extreme to

> me as you might end up memcpy 9k of data just to let someone peek at the

> Ethernet header...

> Thanks,

> Gilad

> 

> Gilad Ben-Yossef

> Software Architect

> EZchip Technologies Ltd.

> 37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel

> Tel: +972-4-959-6666 ext. 576, Fax: +972-8-681-1483

> Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388

> Email: giladb@ezchip.com, Web: http://www.ezchip.com

> 

> "Ethernet always wins."

>         — Andy Bechtolsheim

> 

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

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

> > bounces@lists.linaro.org] On Behalf Of Petri Savolainen

> > Sent: Friday, October 03, 2014 1:18 PM

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

> > Subject: [lng-odp] [PATCH 2/2] Rename packet start function

> >

> > - Unify function naming between packets and segments: xxx_addr(),

> > xxx_data()

> > - Rename odp_packet_start() to odp_packet_data()

> >

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

> > ---

> >  example/ipsec/odp_ipsec_stream.c                |  4 ++--

> >  platform/linux-generic/include/api/odp_packet.h | 32 +++++++++++-------

> -

> > ------

> >  platform/linux-generic/odp_packet.c             |  8 +++----

> >  3 files changed, 20 insertions(+), 24 deletions(-)

> >

> > diff --git a/example/ipsec/odp_ipsec_stream.c

> > b/example/ipsec/odp_ipsec_stream.c

> > index de8305a..fba425c 100644

> > --- a/example/ipsec/odp_ipsec_stream.c

> > +++ b/example/ipsec/odp_ipsec_stream.c

> > @@ -193,8 +193,8 @@ odp_packet_t create_ipv4_packet(stream_db_entry_t

> > *stream,

> >  		return ODP_PACKET_INVALID;

> >  	pkt = odp_packet_from_buffer(bfr);

> >  	odp_packet_init(pkt);

> > -	base = odp_packet_start(pkt);

> > -	data = odp_packet_start(pkt);

> > +	base = odp_packet_data(pkt);

> > +	data = odp_packet_data(pkt);

> >

> >  	/* Ethernet */

> >  	odp_packet_set_inflag_eth(pkt, 1);

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

> > b/platform/linux-generic/include/api/odp_packet.h

> > index 83f345a..316279b 100644

> > --- a/platform/linux-generic/include/api/odp_packet.h

> > +++ b/platform/linux-generic/include/api/odp_packet.h

> > @@ -116,41 +116,37 @@ void odp_packet_set_ctx(odp_packet_t buf, const

> > void *ctx);

> >  void *odp_packet_get_ctx(odp_packet_t buf);

> >

> >  /**

> > - * Get address to the start of the packet buffer

> > + * Packet buffer start address

> > + *

> > + * Returns a pointer to the start of the packet buffer. The address is

> > not

> > + * necessarily the same as packet data address. E.g. on a received

> > Ethernet

> > + * frame, the protocol header may start 2 or 6 bytes within the buffer

> > to

> > + * ensure 32 or 64-bit alignment of the IP header.

> >   *

> > - * The address of the packet buffer is not necessarily the same as the

> > start

> > - * address of the received frame, e.g. an eth frame may be offset by 2

> > or 6

> > - * bytes to ensure 32 or 64-bit alignment of the IP header.

> >   * Use odp_packet_l2(pkt) to get the start address of a received valid

> > frame

> > - * or odp_packet_start(pkt) to get the start address even if no valid

> L2

> > header

> > - * could be found.

> > + * or odp_packet_data(pkt) to get the current packet data address.

> >   *

> >   * @param pkt  Packet handle

> >   *

> >   * @return  Pointer to the start of the packet buffer

> >   *

> > - * @see odp_packet_l2(), odp_packet_start()

> > + * @see odp_packet_l2(), odp_packet_data()

> >   */

> >  uint8_t *odp_packet_addr(odp_packet_t pkt);

> >

> >  /**

> > - * Get pointer to the start of the received frame

> > - *

> > - * The address of the packet buffer is not necessarily the same as the

> > start

> > - * address of the received frame, e.g. an eth frame may be offset by 2

> > or 6

> > - * bytes to ensure 32 or 64-bit alignment of the IP header.

> > - * Use odp_packet_l2(pkt) to get the start address of a received valid

> > eth frame

> > + * Packet data address

> >   *

> > - * odp_packet_start() will always return a pointer to the start of the

> > frame,

> > - * even if the frame is unrecognized and no valid L2 header could be

> > found.

> > + * Returns the current packet data address. When a packet is received

> > from

> > + * packet input, the data address points to the first byte of the

> > packet.

> >   *

> >   * @param pkt  Packet handle

> >   *

> > - * @return  Pointer to the start of the received frame

> > + * @return  Pointer to the packet data

> >   *

> >   * @see odp_packet_l2(), odp_packet_addr()

> >   */

> > -uint8_t *odp_packet_start(odp_packet_t pkt);

> > +uint8_t *odp_packet_data(odp_packet_t pkt);

> >

> >  /**

> >   * Get pointer to the start of the L2 frame

> > @@ -162,7 +158,7 @@ uint8_t *odp_packet_start(odp_packet_t pkt);

> >   *

> >   * @return  Pointer to L2 header or NULL if not found

> >   *

> > - * @see odp_packet_addr(), odp_packet_start()

> > + * @see odp_packet_addr(), odp_packet_data()

> >   */

> >  uint8_t *odp_packet_l2(odp_packet_t pkt);

> >

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

> > generic/odp_packet.c

> > index 6cd6183..435bc8b 100644

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

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

> > @@ -61,7 +61,7 @@ uint8_t *odp_packet_addr(odp_packet_t pkt)

> >  	return odp_buffer_addr(odp_buffer_from_packet(pkt));

> >  }

> >

> > -uint8_t *odp_packet_start(odp_packet_t pkt)

> > +uint8_t *odp_packet_data(odp_packet_t pkt)

> >  {

> >  	return odp_packet_addr(pkt) + odp_packet_hdr(pkt)->frame_offset;

> >  }

> > @@ -183,7 +183,7 @@ void odp_packet_parse(odp_packet_t pkt, size_t len,

> > size_t frame_offset)

> >  	pkt_hdr->input_flags.l2 = 1;

> >  	pkt_hdr->l2_offset = frame_offset;

> >

> > -	eth = (odph_ethhdr_t *)odp_packet_start(pkt);

> > +	eth = (odph_ethhdr_t *)odp_packet_data(pkt);

> >  	ethtype = odp_be_to_cpu_16(eth->type);

> >  	vlan = (odph_vlanhdr_t *)&eth->type;

> >

> > @@ -372,8 +372,8 @@ int odp_packet_copy(odp_packet_t pkt_dst,

> > odp_packet_t pkt_src)

> >  	memcpy(start_dst, start_src, len);

> >

> >  	/* Copy frame payload */

> > -	start_dst = (uint8_t *)odp_packet_start(pkt_dst);

> > -	start_src = (uint8_t *)odp_packet_start(pkt_src);

> > +	start_dst = (uint8_t *)odp_packet_data(pkt_dst);

> > +	start_src = (uint8_t *)odp_packet_data(pkt_src);

> >  	len = pkt_hdr_src->frame_len;

> >  	memcpy(start_dst, start_src, len);

> >

> > --

> > 2.1.1

> >

> >

> > _______________________________________________

> > lng-odp mailing list

> > lng-odp@lists.linaro.org

> > http://lists.linaro.org/mailman/listinfo/lng-odp
Gilad Ben-Yossef Oct. 6, 2014, 8:41 a.m. UTC | #7
OK, if there is no expectation that these APIs work on segmented packets than that answers my question.
Thanks,
Gilad



Gilad Ben-Yossef
Software Architect
EZchip Technologies Ltd.
37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel
Tel: +972-4-959-6666 ext. 576, Fax: +972-8-681-1483 
Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388
Email: giladb@ezchip.com, Web: http://www.ezchip.com

"Ethernet always wins."
        — Andy Bechtolsheim


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

> From: Savolainen, Petri (NSN - FI/Espoo)

> [mailto:petri.savolainen@nsn.com]

> Sent: Monday, October 06, 2014 11:05 AM

> To: Gilad Ben Yossef; Petri Savolainen; lng-odp@lists.linaro.org

> Subject: RE: [lng-odp] [PATCH 2/2] Rename packet start function

> 

> Hi Gilad,

> 

> 

> odp_packet_is_segmented() will tell the user if a packet is segmented. If

> it's segmented, there are a series of packet segment functions

> (odp_packet_seg_xxx) to apply on individual segments. Packet level

> functions (odp_packet_xxx) is used when packet is not segmented. I'm

> renaming/modifying the packet level size/len functions next. The naming

> logic is the following:

> 

> xxx_addr     = buffer start address

> xxx_size     = buffer size

> xxx_data     = pointer to the data within the buffer

> xxx_data_len = data length

> 

> data and data_len cannot be set directly, but can be moved with specific

> functions (see odp_packet_seg_push/pull in current odp_packet.h).

> 

> headroom len = data - addr

> tailroom len = size - headroom - data_len

> 

> 

> -Petri

> 

> 

> 

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

> > From: ext Gilad Ben Yossef [mailto:giladb@ezchip.com]

> > Sent: Monday, October 06, 2014 9:08 AM

> > To: Petri Savolainen; lng-odp@lists.linaro.org

> > Subject: RE: [lng-odp] [PATCH 2/2] Rename packet start function

> >

> > Hi Petri,

> > Thanks for the rename.

> > While we're touching the packet/buffer data access APIs - I'm a little

> > puzzled on how are these APIs supposed to work if the packet is

> segmented?

> > Let's say odp_packet_data() handed you a pointer to packet data of a

> > segmented packet - how do you know how much data is there that you can

> > access?

> > Is the expectation that every call to odp_packet_data() will copy all

> > segmented packet data to make them contiguous? This sound a bit extreme

> to

> > me as you might end up memcpy 9k of data just to let someone peek at

> the

> > Ethernet header...

> > Thanks,

> > Gilad

> >

> > Gilad Ben-Yossef

> > Software Architect

> > EZchip Technologies Ltd.

> > 37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel

> > Tel: +972-4-959-6666 ext. 576, Fax: +972-8-681-1483

> > Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388

> > Email: giladb@ezchip.com, Web: http://www.ezchip.com

> >

> > "Ethernet always wins."

> >         — Andy Bechtolsheim

> >

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

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

> > > bounces@lists.linaro.org] On Behalf Of Petri Savolainen

> > > Sent: Friday, October 03, 2014 1:18 PM

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

> > > Subject: [lng-odp] [PATCH 2/2] Rename packet start function

> > >

> > > - Unify function naming between packets and segments: xxx_addr(),

> > > xxx_data()

> > > - Rename odp_packet_start() to odp_packet_data()

> > >

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

> > > ---

> > >  example/ipsec/odp_ipsec_stream.c                |  4 ++--

> > >  platform/linux-generic/include/api/odp_packet.h | 32 +++++++++++----

> ---

> > -

> > > ------

> > >  platform/linux-generic/odp_packet.c             |  8 +++----

> > >  3 files changed, 20 insertions(+), 24 deletions(-)

> > >

> > > diff --git a/example/ipsec/odp_ipsec_stream.c

> > > b/example/ipsec/odp_ipsec_stream.c

> > > index de8305a..fba425c 100644

> > > --- a/example/ipsec/odp_ipsec_stream.c

> > > +++ b/example/ipsec/odp_ipsec_stream.c

> > > @@ -193,8 +193,8 @@ odp_packet_t create_ipv4_packet(stream_db_entry_t

> > > *stream,

> > >  		return ODP_PACKET_INVALID;

> > >  	pkt = odp_packet_from_buffer(bfr);

> > >  	odp_packet_init(pkt);

> > > -	base = odp_packet_start(pkt);

> > > -	data = odp_packet_start(pkt);

> > > +	base = odp_packet_data(pkt);

> > > +	data = odp_packet_data(pkt);

> > >

> > >  	/* Ethernet */

> > >  	odp_packet_set_inflag_eth(pkt, 1);

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

> > > b/platform/linux-generic/include/api/odp_packet.h

> > > index 83f345a..316279b 100644

> > > --- a/platform/linux-generic/include/api/odp_packet.h

> > > +++ b/platform/linux-generic/include/api/odp_packet.h

> > > @@ -116,41 +116,37 @@ void odp_packet_set_ctx(odp_packet_t buf, const

> > > void *ctx);

> > >  void *odp_packet_get_ctx(odp_packet_t buf);

> > >

> > >  /**

> > > - * Get address to the start of the packet buffer

> > > + * Packet buffer start address

> > > + *

> > > + * Returns a pointer to the start of the packet buffer. The address

> is

> > > not

> > > + * necessarily the same as packet data address. E.g. on a received

> > > Ethernet

> > > + * frame, the protocol header may start 2 or 6 bytes within the

> buffer

> > > to

> > > + * ensure 32 or 64-bit alignment of the IP header.

> > >   *

> > > - * The address of the packet buffer is not necessarily the same as

> the

> > > start

> > > - * address of the received frame, e.g. an eth frame may be offset by

> 2

> > > or 6

> > > - * bytes to ensure 32 or 64-bit alignment of the IP header.

> > >   * Use odp_packet_l2(pkt) to get the start address of a received

> valid

> > > frame

> > > - * or odp_packet_start(pkt) to get the start address even if no

> valid

> > L2

> > > header

> > > - * could be found.

> > > + * or odp_packet_data(pkt) to get the current packet data address.

> > >   *

> > >   * @param pkt  Packet handle

> > >   *

> > >   * @return  Pointer to the start of the packet buffer

> > >   *

> > > - * @see odp_packet_l2(), odp_packet_start()

> > > + * @see odp_packet_l2(), odp_packet_data()

> > >   */

> > >  uint8_t *odp_packet_addr(odp_packet_t pkt);

> > >

> > >  /**

> > > - * Get pointer to the start of the received frame

> > > - *

> > > - * The address of the packet buffer is not necessarily the same as

> the

> > > start

> > > - * address of the received frame, e.g. an eth frame may be offset by

> 2

> > > or 6

> > > - * bytes to ensure 32 or 64-bit alignment of the IP header.

> > > - * Use odp_packet_l2(pkt) to get the start address of a received

> valid

> > > eth frame

> > > + * Packet data address

> > >   *

> > > - * odp_packet_start() will always return a pointer to the start of

> the

> > > frame,

> > > - * even if the frame is unrecognized and no valid L2 header could be

> > > found.

> > > + * Returns the current packet data address. When a packet is

> received

> > > from

> > > + * packet input, the data address points to the first byte of the

> > > packet.

> > >   *

> > >   * @param pkt  Packet handle

> > >   *

> > > - * @return  Pointer to the start of the received frame

> > > + * @return  Pointer to the packet data

> > >   *

> > >   * @see odp_packet_l2(), odp_packet_addr()

> > >   */

> > > -uint8_t *odp_packet_start(odp_packet_t pkt);

> > > +uint8_t *odp_packet_data(odp_packet_t pkt);

> > >

> > >  /**

> > >   * Get pointer to the start of the L2 frame

> > > @@ -162,7 +158,7 @@ uint8_t *odp_packet_start(odp_packet_t pkt);

> > >   *

> > >   * @return  Pointer to L2 header or NULL if not found

> > >   *

> > > - * @see odp_packet_addr(), odp_packet_start()

> > > + * @see odp_packet_addr(), odp_packet_data()

> > >   */

> > >  uint8_t *odp_packet_l2(odp_packet_t pkt);

> > >

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

> > > generic/odp_packet.c

> > > index 6cd6183..435bc8b 100644

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

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

> > > @@ -61,7 +61,7 @@ uint8_t *odp_packet_addr(odp_packet_t pkt)

> > >  	return odp_buffer_addr(odp_buffer_from_packet(pkt));

> > >  }

> > >

> > > -uint8_t *odp_packet_start(odp_packet_t pkt)

> > > +uint8_t *odp_packet_data(odp_packet_t pkt)

> > >  {

> > >  	return odp_packet_addr(pkt) + odp_packet_hdr(pkt)->frame_offset;

> > >  }

> > > @@ -183,7 +183,7 @@ void odp_packet_parse(odp_packet_t pkt, size_t

> len,

> > > size_t frame_offset)

> > >  	pkt_hdr->input_flags.l2 = 1;

> > >  	pkt_hdr->l2_offset = frame_offset;

> > >

> > > -	eth = (odph_ethhdr_t *)odp_packet_start(pkt);

> > > +	eth = (odph_ethhdr_t *)odp_packet_data(pkt);

> > >  	ethtype = odp_be_to_cpu_16(eth->type);

> > >  	vlan = (odph_vlanhdr_t *)&eth->type;

> > >

> > > @@ -372,8 +372,8 @@ int odp_packet_copy(odp_packet_t pkt_dst,

> > > odp_packet_t pkt_src)

> > >  	memcpy(start_dst, start_src, len);

> > >

> > >  	/* Copy frame payload */

> > > -	start_dst = (uint8_t *)odp_packet_start(pkt_dst);

> > > -	start_src = (uint8_t *)odp_packet_start(pkt_src);

> > > +	start_dst = (uint8_t *)odp_packet_data(pkt_dst);

> > > +	start_src = (uint8_t *)odp_packet_data(pkt_src);

> > >  	len = pkt_hdr_src->frame_len;

> > >  	memcpy(start_dst, start_src, len);

> > >

> > > --

> > > 2.1.1

> > >

> > >

> > > _______________________________________________

> > > lng-odp mailing list

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

> > > http://lists.linaro.org/mailman/listinfo/lng-odp
Jerin Jacob Oct. 6, 2014, 9:04 a.m. UTC | #8
On Mon, Oct 06, 2014 at 08:41:54AM +0000, Gilad Ben Yossef wrote:
> OK, if there is no expectation that these APIs work on segmented packets than that answers my question.
> Thanks,
> Gilad
> 
> 
> 
> Gilad Ben-Yossef
> Software Architect
> EZchip Technologies Ltd.
> 37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel
> Tel: +972-4-959-6666 ext. 576, Fax: +972-8-681-1483 
> Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388
> Email: giladb@ezchip.com, Web: http://www.ezchip.com
> 
> "Ethernet always wins."
>         — Andy Bechtolsheim
> 
> 
> > -----Original Message-----
> > From: Savolainen, Petri (NSN - FI/Espoo)
> > [mailto:petri.savolainen@nsn.com]
> > Sent: Monday, October 06, 2014 11:05 AM
> > To: Gilad Ben Yossef; Petri Savolainen; lng-odp@lists.linaro.org
> > Subject: RE: [lng-odp] [PATCH 2/2] Rename packet start function
> > 
> > Hi Gilad,
> > 
> > 
> > odp_packet_is_segmented() will tell the user if a packet is segmented. If
> > it's segmented, there are a series of packet segment functions
> > (odp_packet_seg_xxx) to apply on individual segments. Packet level
> > functions (odp_packet_xxx) is used when packet is not segmented. I'm

So how does an ODP application programmer use the functions like "odp_packet_l3_offset" when packets are segmented ? 
for instance application want to know the l4 protocol from l3 header etc ?


> > renaming/modifying the packet level size/len functions next. The naming
> > logic is the following:
> > 
> > xxx_addr     = buffer start address
> > xxx_size     = buffer size
> > xxx_data     = pointer to the data within the buffer
> > xxx_data_len = data length
> > 
> > data and data_len cannot be set directly, but can be moved with specific
> > functions (see odp_packet_seg_push/pull in current odp_packet.h).
> > 
> > headroom len = data - addr
> > tailroom len = size - headroom - data_len
> > 
> > 
> > -Petri
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: ext Gilad Ben Yossef [mailto:giladb@ezchip.com]
> > > Sent: Monday, October 06, 2014 9:08 AM
> > > To: Petri Savolainen; lng-odp@lists.linaro.org
> > > Subject: RE: [lng-odp] [PATCH 2/2] Rename packet start function
> > >
> > > Hi Petri,
> > > Thanks for the rename.
> > > While we're touching the packet/buffer data access APIs - I'm a little
> > > puzzled on how are these APIs supposed to work if the packet is
> > segmented?
> > > Let's say odp_packet_data() handed you a pointer to packet data of a
> > > segmented packet - how do you know how much data is there that you can
> > > access?
> > > Is the expectation that every call to odp_packet_data() will copy all
> > > segmented packet data to make them contiguous? This sound a bit extreme
> > to
> > > me as you might end up memcpy 9k of data just to let someone peek at
> > the
> > > Ethernet header...
> > > Thanks,
> > > Gilad
> > >
> > > Gilad Ben-Yossef
> > > Software Architect
> > > EZchip Technologies Ltd.
> > > 37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel
> > > Tel: +972-4-959-6666 ext. 576, Fax: +972-8-681-1483
> > > Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388
> > > Email: giladb@ezchip.com, Web: http://www.ezchip.com
> > >
> > > "Ethernet always wins."
> > >         — Andy Bechtolsheim
> > >
> > > > -----Original Message-----
> > > > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> > > > bounces@lists.linaro.org] On Behalf Of Petri Savolainen
> > > > Sent: Friday, October 03, 2014 1:18 PM
> > > > To: lng-odp@lists.linaro.org
> > > > Subject: [lng-odp] [PATCH 2/2] Rename packet start function
> > > >
> > > > - Unify function naming between packets and segments: xxx_addr(),
> > > > xxx_data()
> > > > - Rename odp_packet_start() to odp_packet_data()
> > > >
> > > > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>
> > > > ---
> > > >  example/ipsec/odp_ipsec_stream.c                |  4 ++--
> > > >  platform/linux-generic/include/api/odp_packet.h | 32 +++++++++++----
> > ---
> > > -
> > > > ------
> > > >  platform/linux-generic/odp_packet.c             |  8 +++----
> > > >  3 files changed, 20 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/example/ipsec/odp_ipsec_stream.c
> > > > b/example/ipsec/odp_ipsec_stream.c
> > > > index de8305a..fba425c 100644
> > > > --- a/example/ipsec/odp_ipsec_stream.c
> > > > +++ b/example/ipsec/odp_ipsec_stream.c
> > > > @@ -193,8 +193,8 @@ odp_packet_t create_ipv4_packet(stream_db_entry_t
> > > > *stream,
> > > >  		return ODP_PACKET_INVALID;
> > > >  	pkt = odp_packet_from_buffer(bfr);
> > > >  	odp_packet_init(pkt);
> > > > -	base = odp_packet_start(pkt);
> > > > -	data = odp_packet_start(pkt);
> > > > +	base = odp_packet_data(pkt);
> > > > +	data = odp_packet_data(pkt);
> > > >
> > > >  	/* Ethernet */
> > > >  	odp_packet_set_inflag_eth(pkt, 1);
> > > > diff --git a/platform/linux-generic/include/api/odp_packet.h
> > > > b/platform/linux-generic/include/api/odp_packet.h
> > > > index 83f345a..316279b 100644
> > > > --- a/platform/linux-generic/include/api/odp_packet.h
> > > > +++ b/platform/linux-generic/include/api/odp_packet.h
> > > > @@ -116,41 +116,37 @@ void odp_packet_set_ctx(odp_packet_t buf, const
> > > > void *ctx);
> > > >  void *odp_packet_get_ctx(odp_packet_t buf);
> > > >
> > > >  /**
> > > > - * Get address to the start of the packet buffer
> > > > + * Packet buffer start address
> > > > + *
> > > > + * Returns a pointer to the start of the packet buffer. The address
> > is
> > > > not
> > > > + * necessarily the same as packet data address. E.g. on a received
> > > > Ethernet
> > > > + * frame, the protocol header may start 2 or 6 bytes within the
> > buffer
> > > > to
> > > > + * ensure 32 or 64-bit alignment of the IP header.
> > > >   *
> > > > - * The address of the packet buffer is not necessarily the same as
> > the
> > > > start
> > > > - * address of the received frame, e.g. an eth frame may be offset by
> > 2
> > > > or 6
> > > > - * bytes to ensure 32 or 64-bit alignment of the IP header.
> > > >   * Use odp_packet_l2(pkt) to get the start address of a received
> > valid
> > > > frame
> > > > - * or odp_packet_start(pkt) to get the start address even if no
> > valid
> > > L2
> > > > header
> > > > - * could be found.
> > > > + * or odp_packet_data(pkt) to get the current packet data address.
> > > >   *
> > > >   * @param pkt  Packet handle
> > > >   *
> > > >   * @return  Pointer to the start of the packet buffer
> > > >   *
> > > > - * @see odp_packet_l2(), odp_packet_start()
> > > > + * @see odp_packet_l2(), odp_packet_data()
> > > >   */
> > > >  uint8_t *odp_packet_addr(odp_packet_t pkt);
> > > >
> > > >  /**
> > > > - * Get pointer to the start of the received frame
> > > > - *
> > > > - * The address of the packet buffer is not necessarily the same as
> > the
> > > > start
> > > > - * address of the received frame, e.g. an eth frame may be offset by
> > 2
> > > > or 6
> > > > - * bytes to ensure 32 or 64-bit alignment of the IP header.
> > > > - * Use odp_packet_l2(pkt) to get the start address of a received
> > valid
> > > > eth frame
> > > > + * Packet data address
> > > >   *
> > > > - * odp_packet_start() will always return a pointer to the start of
> > the
> > > > frame,
> > > > - * even if the frame is unrecognized and no valid L2 header could be
> > > > found.
> > > > + * Returns the current packet data address. When a packet is
> > received
> > > > from
> > > > + * packet input, the data address points to the first byte of the
> > > > packet.
> > > >   *
> > > >   * @param pkt  Packet handle
> > > >   *
> > > > - * @return  Pointer to the start of the received frame
> > > > + * @return  Pointer to the packet data
> > > >   *
> > > >   * @see odp_packet_l2(), odp_packet_addr()
> > > >   */
> > > > -uint8_t *odp_packet_start(odp_packet_t pkt);
> > > > +uint8_t *odp_packet_data(odp_packet_t pkt);
> > > >
> > > >  /**
> > > >   * Get pointer to the start of the L2 frame
> > > > @@ -162,7 +158,7 @@ uint8_t *odp_packet_start(odp_packet_t pkt);
> > > >   *
> > > >   * @return  Pointer to L2 header or NULL if not found
> > > >   *
> > > > - * @see odp_packet_addr(), odp_packet_start()
> > > > + * @see odp_packet_addr(), odp_packet_data()
> > > >   */
> > > >  uint8_t *odp_packet_l2(odp_packet_t pkt);
> > > >
> > > > diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-
> > > > generic/odp_packet.c
> > > > index 6cd6183..435bc8b 100644
> > > > --- a/platform/linux-generic/odp_packet.c
> > > > +++ b/platform/linux-generic/odp_packet.c
> > > > @@ -61,7 +61,7 @@ uint8_t *odp_packet_addr(odp_packet_t pkt)
> > > >  	return odp_buffer_addr(odp_buffer_from_packet(pkt));
> > > >  }
> > > >
> > > > -uint8_t *odp_packet_start(odp_packet_t pkt)
> > > > +uint8_t *odp_packet_data(odp_packet_t pkt)
> > > >  {
> > > >  	return odp_packet_addr(pkt) + odp_packet_hdr(pkt)->frame_offset;
> > > >  }
> > > > @@ -183,7 +183,7 @@ void odp_packet_parse(odp_packet_t pkt, size_t
> > len,
> > > > size_t frame_offset)
> > > >  	pkt_hdr->input_flags.l2 = 1;
> > > >  	pkt_hdr->l2_offset = frame_offset;
> > > >
> > > > -	eth = (odph_ethhdr_t *)odp_packet_start(pkt);
> > > > +	eth = (odph_ethhdr_t *)odp_packet_data(pkt);
> > > >  	ethtype = odp_be_to_cpu_16(eth->type);
> > > >  	vlan = (odph_vlanhdr_t *)&eth->type;
> > > >
> > > > @@ -372,8 +372,8 @@ int odp_packet_copy(odp_packet_t pkt_dst,
> > > > odp_packet_t pkt_src)
> > > >  	memcpy(start_dst, start_src, len);
> > > >
> > > >  	/* Copy frame payload */
> > > > -	start_dst = (uint8_t *)odp_packet_start(pkt_dst);
> > > > -	start_src = (uint8_t *)odp_packet_start(pkt_src);
> > > > +	start_dst = (uint8_t *)odp_packet_data(pkt_dst);
> > > > +	start_src = (uint8_t *)odp_packet_data(pkt_src);
> > > >  	len = pkt_hdr_src->frame_len;
> > > >  	memcpy(start_dst, start_src, len);
> > > >
> > > > --
> > > > 2.1.1
> > > >
> > > >
> > > > _______________________________________________
> > > > lng-odp mailing list
> > > > lng-odp@lists.linaro.org
> > > > http://lists.linaro.org/mailman/listinfo/lng-odp
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
Bill Fischofer Oct. 6, 2014, 10:37 a.m. UTC | #9
Proposals for how to deal with all of this are contained in the ODP Packet
API design doc I circulated last week.  We'll be discussing this during
tomorrow's ODP call.

Bill

On Mon, Oct 6, 2014 at 4:04 AM, Jerin Jacob <jerin.jacob@caviumnetworks.com>
wrote:

> On Mon, Oct 06, 2014 at 08:41:54AM +0000, Gilad Ben Yossef wrote:
> > OK, if there is no expectation that these APIs work on segmented packets
> than that answers my question.
> > Thanks,
> > Gilad
> >
> >
> >
> > Gilad Ben-Yossef
> > Software Architect
> > EZchip Technologies Ltd.
> > 37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel
> > Tel: +972-4-959-6666 ext. 576, Fax: +972-8-681-1483
> > Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388
> > Email: giladb@ezchip.com, Web: http://www.ezchip.com
> >
> > "Ethernet always wins."
> >         — Andy Bechtolsheim
> >
> >
> > > -----Original Message-----
> > > From: Savolainen, Petri (NSN - FI/Espoo)
> > > [mailto:petri.savolainen@nsn.com]
> > > Sent: Monday, October 06, 2014 11:05 AM
> > > To: Gilad Ben Yossef; Petri Savolainen; lng-odp@lists.linaro.org
> > > Subject: RE: [lng-odp] [PATCH 2/2] Rename packet start function
> > >
> > > Hi Gilad,
> > >
> > >
> > > odp_packet_is_segmented() will tell the user if a packet is segmented.
> If
> > > it's segmented, there are a series of packet segment functions
> > > (odp_packet_seg_xxx) to apply on individual segments. Packet level
> > > functions (odp_packet_xxx) is used when packet is not segmented. I'm
>
> So how does an ODP application programmer use the functions like
> "odp_packet_l3_offset" when packets are segmented ?
> for instance application want to know the l4 protocol from l3 header etc ?
>
>
> > > renaming/modifying the packet level size/len functions next. The naming
> > > logic is the following:
> > >
> > > xxx_addr     = buffer start address
> > > xxx_size     = buffer size
> > > xxx_data     = pointer to the data within the buffer
> > > xxx_data_len = data length
> > >
> > > data and data_len cannot be set directly, but can be moved with
> specific
> > > functions (see odp_packet_seg_push/pull in current odp_packet.h).
> > >
> > > headroom len = data - addr
> > > tailroom len = size - headroom - data_len
> > >
> > >
> > > -Petri
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: ext Gilad Ben Yossef [mailto:giladb@ezchip.com]
> > > > Sent: Monday, October 06, 2014 9:08 AM
> > > > To: Petri Savolainen; lng-odp@lists.linaro.org
> > > > Subject: RE: [lng-odp] [PATCH 2/2] Rename packet start function
> > > >
> > > > Hi Petri,
> > > > Thanks for the rename.
> > > > While we're touching the packet/buffer data access APIs - I'm a
> little
> > > > puzzled on how are these APIs supposed to work if the packet is
> > > segmented?
> > > > Let's say odp_packet_data() handed you a pointer to packet data of a
> > > > segmented packet - how do you know how much data is there that you
> can
> > > > access?
> > > > Is the expectation that every call to odp_packet_data() will copy all
> > > > segmented packet data to make them contiguous? This sound a bit
> extreme
> > > to
> > > > me as you might end up memcpy 9k of data just to let someone peek at
> > > the
> > > > Ethernet header...
> > > > Thanks,
> > > > Gilad
> > > >
> > > > Gilad Ben-Yossef
> > > > Software Architect
> > > > EZchip Technologies Ltd.
> > > > 37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel
> > > > Tel: +972-4-959-6666 ext. 576, Fax: +972-8-681-1483
> > > > Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388
> > > > Email: giladb@ezchip.com, Web: http://www.ezchip.com
> > > >
> > > > "Ethernet always wins."
> > > >         — Andy Bechtolsheim
> > > >
> > > > > -----Original Message-----
> > > > > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> > > > > bounces@lists.linaro.org] On Behalf Of Petri Savolainen
> > > > > Sent: Friday, October 03, 2014 1:18 PM
> > > > > To: lng-odp@lists.linaro.org
> > > > > Subject: [lng-odp] [PATCH 2/2] Rename packet start function
> > > > >
> > > > > - Unify function naming between packets and segments: xxx_addr(),
> > > > > xxx_data()
> > > > > - Rename odp_packet_start() to odp_packet_data()
> > > > >
> > > > > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>
> > > > > ---
> > > > >  example/ipsec/odp_ipsec_stream.c                |  4 ++--
> > > > >  platform/linux-generic/include/api/odp_packet.h | 32
> +++++++++++----
> > > ---
> > > > -
> > > > > ------
> > > > >  platform/linux-generic/odp_packet.c             |  8 +++----
> > > > >  3 files changed, 20 insertions(+), 24 deletions(-)
> > > > >
> > > > > diff --git a/example/ipsec/odp_ipsec_stream.c
> > > > > b/example/ipsec/odp_ipsec_stream.c
> > > > > index de8305a..fba425c 100644
> > > > > --- a/example/ipsec/odp_ipsec_stream.c
> > > > > +++ b/example/ipsec/odp_ipsec_stream.c
> > > > > @@ -193,8 +193,8 @@ odp_packet_t
> create_ipv4_packet(stream_db_entry_t
> > > > > *stream,
> > > > >                 return ODP_PACKET_INVALID;
> > > > >         pkt = odp_packet_from_buffer(bfr);
> > > > >         odp_packet_init(pkt);
> > > > > -       base = odp_packet_start(pkt);
> > > > > -       data = odp_packet_start(pkt);
> > > > > +       base = odp_packet_data(pkt);
> > > > > +       data = odp_packet_data(pkt);
> > > > >
> > > > >         /* Ethernet */
> > > > >         odp_packet_set_inflag_eth(pkt, 1);
> > > > > diff --git a/platform/linux-generic/include/api/odp_packet.h
> > > > > b/platform/linux-generic/include/api/odp_packet.h
> > > > > index 83f345a..316279b 100644
> > > > > --- a/platform/linux-generic/include/api/odp_packet.h
> > > > > +++ b/platform/linux-generic/include/api/odp_packet.h
> > > > > @@ -116,41 +116,37 @@ void odp_packet_set_ctx(odp_packet_t buf,
> const
> > > > > void *ctx);
> > > > >  void *odp_packet_get_ctx(odp_packet_t buf);
> > > > >
> > > > >  /**
> > > > > - * Get address to the start of the packet buffer
> > > > > + * Packet buffer start address
> > > > > + *
> > > > > + * Returns a pointer to the start of the packet buffer. The
> address
> > > is
> > > > > not
> > > > > + * necessarily the same as packet data address. E.g. on a received
> > > > > Ethernet
> > > > > + * frame, the protocol header may start 2 or 6 bytes within the
> > > buffer
> > > > > to
> > > > > + * ensure 32 or 64-bit alignment of the IP header.
> > > > >   *
> > > > > - * The address of the packet buffer is not necessarily the same as
> > > the
> > > > > start
> > > > > - * address of the received frame, e.g. an eth frame may be offset
> by
> > > 2
> > > > > or 6
> > > > > - * bytes to ensure 32 or 64-bit alignment of the IP header.
> > > > >   * Use odp_packet_l2(pkt) to get the start address of a received
> > > valid
> > > > > frame
> > > > > - * or odp_packet_start(pkt) to get the start address even if no
> > > valid
> > > > L2
> > > > > header
> > > > > - * could be found.
> > > > > + * or odp_packet_data(pkt) to get the current packet data address.
> > > > >   *
> > > > >   * @param pkt  Packet handle
> > > > >   *
> > > > >   * @return  Pointer to the start of the packet buffer
> > > > >   *
> > > > > - * @see odp_packet_l2(), odp_packet_start()
> > > > > + * @see odp_packet_l2(), odp_packet_data()
> > > > >   */
> > > > >  uint8_t *odp_packet_addr(odp_packet_t pkt);
> > > > >
> > > > >  /**
> > > > > - * Get pointer to the start of the received frame
> > > > > - *
> > > > > - * The address of the packet buffer is not necessarily the same as
> > > the
> > > > > start
> > > > > - * address of the received frame, e.g. an eth frame may be offset
> by
> > > 2
> > > > > or 6
> > > > > - * bytes to ensure 32 or 64-bit alignment of the IP header.
> > > > > - * Use odp_packet_l2(pkt) to get the start address of a received
> > > valid
> > > > > eth frame
> > > > > + * Packet data address
> > > > >   *
> > > > > - * odp_packet_start() will always return a pointer to the start of
> > > the
> > > > > frame,
> > > > > - * even if the frame is unrecognized and no valid L2 header could
> be
> > > > > found.
> > > > > + * Returns the current packet data address. When a packet is
> > > received
> > > > > from
> > > > > + * packet input, the data address points to the first byte of the
> > > > > packet.
> > > > >   *
> > > > >   * @param pkt  Packet handle
> > > > >   *
> > > > > - * @return  Pointer to the start of the received frame
> > > > > + * @return  Pointer to the packet data
> > > > >   *
> > > > >   * @see odp_packet_l2(), odp_packet_addr()
> > > > >   */
> > > > > -uint8_t *odp_packet_start(odp_packet_t pkt);
> > > > > +uint8_t *odp_packet_data(odp_packet_t pkt);
> > > > >
> > > > >  /**
> > > > >   * Get pointer to the start of the L2 frame
> > > > > @@ -162,7 +158,7 @@ uint8_t *odp_packet_start(odp_packet_t pkt);
> > > > >   *
> > > > >   * @return  Pointer to L2 header or NULL if not found
> > > > >   *
> > > > > - * @see odp_packet_addr(), odp_packet_start()
> > > > > + * @see odp_packet_addr(), odp_packet_data()
> > > > >   */
> > > > >  uint8_t *odp_packet_l2(odp_packet_t pkt);
> > > > >
> > > > > diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-
> > > > > generic/odp_packet.c
> > > > > index 6cd6183..435bc8b 100644
> > > > > --- a/platform/linux-generic/odp_packet.c
> > > > > +++ b/platform/linux-generic/odp_packet.c
> > > > > @@ -61,7 +61,7 @@ uint8_t *odp_packet_addr(odp_packet_t pkt)
> > > > >         return odp_buffer_addr(odp_buffer_from_packet(pkt));
> > > > >  }
> > > > >
> > > > > -uint8_t *odp_packet_start(odp_packet_t pkt)
> > > > > +uint8_t *odp_packet_data(odp_packet_t pkt)
> > > > >  {
> > > > >         return odp_packet_addr(pkt) +
> odp_packet_hdr(pkt)->frame_offset;
> > > > >  }
> > > > > @@ -183,7 +183,7 @@ void odp_packet_parse(odp_packet_t pkt, size_t
> > > len,
> > > > > size_t frame_offset)
> > > > >         pkt_hdr->input_flags.l2 = 1;
> > > > >         pkt_hdr->l2_offset = frame_offset;
> > > > >
> > > > > -       eth = (odph_ethhdr_t *)odp_packet_start(pkt);
> > > > > +       eth = (odph_ethhdr_t *)odp_packet_data(pkt);
> > > > >         ethtype = odp_be_to_cpu_16(eth->type);
> > > > >         vlan = (odph_vlanhdr_t *)&eth->type;
> > > > >
> > > > > @@ -372,8 +372,8 @@ int odp_packet_copy(odp_packet_t pkt_dst,
> > > > > odp_packet_t pkt_src)
> > > > >         memcpy(start_dst, start_src, len);
> > > > >
> > > > >         /* Copy frame payload */
> > > > > -       start_dst = (uint8_t *)odp_packet_start(pkt_dst);
> > > > > -       start_src = (uint8_t *)odp_packet_start(pkt_src);
> > > > > +       start_dst = (uint8_t *)odp_packet_data(pkt_dst);
> > > > > +       start_src = (uint8_t *)odp_packet_data(pkt_src);
> > > > >         len = pkt_hdr_src->frame_len;
> > > > >         memcpy(start_dst, start_src, len);
> > > > >
> > > > > --
> > > > > 2.1.1
> > > > >
> > > > >
> > > > > _______________________________________________
> > > > > lng-odp mailing list
> > > > > lng-odp@lists.linaro.org
> > > > > http://lists.linaro.org/mailman/listinfo/lng-odp
> > _______________________________________________
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/lng-odp
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
diff mbox

Patch

diff --git a/example/ipsec/odp_ipsec_stream.c b/example/ipsec/odp_ipsec_stream.c
index de8305a..fba425c 100644
--- a/example/ipsec/odp_ipsec_stream.c
+++ b/example/ipsec/odp_ipsec_stream.c
@@ -193,8 +193,8 @@  odp_packet_t create_ipv4_packet(stream_db_entry_t *stream,
 		return ODP_PACKET_INVALID;
 	pkt = odp_packet_from_buffer(bfr);
 	odp_packet_init(pkt);
-	base = odp_packet_start(pkt);
-	data = odp_packet_start(pkt);
+	base = odp_packet_data(pkt);
+	data = odp_packet_data(pkt);
 
 	/* Ethernet */
 	odp_packet_set_inflag_eth(pkt, 1);
diff --git a/platform/linux-generic/include/api/odp_packet.h b/platform/linux-generic/include/api/odp_packet.h
index 83f345a..316279b 100644
--- a/platform/linux-generic/include/api/odp_packet.h
+++ b/platform/linux-generic/include/api/odp_packet.h
@@ -116,41 +116,37 @@  void odp_packet_set_ctx(odp_packet_t buf, const void *ctx);
 void *odp_packet_get_ctx(odp_packet_t buf);
 
 /**
- * Get address to the start of the packet buffer
+ * Packet buffer start address
+ *
+ * Returns a pointer to the start of the packet buffer. The address is not
+ * necessarily the same as packet data address. E.g. on a received Ethernet
+ * frame, the protocol header may start 2 or 6 bytes within the buffer to
+ * ensure 32 or 64-bit alignment of the IP header.
  *
- * The address of the packet buffer is not necessarily the same as the start
- * address of the received frame, e.g. an eth frame may be offset by 2 or 6
- * bytes to ensure 32 or 64-bit alignment of the IP header.
  * Use odp_packet_l2(pkt) to get the start address of a received valid frame
- * or odp_packet_start(pkt) to get the start address even if no valid L2 header
- * could be found.
+ * or odp_packet_data(pkt) to get the current packet data address.
  *
  * @param pkt  Packet handle
  *
  * @return  Pointer to the start of the packet buffer
  *
- * @see odp_packet_l2(), odp_packet_start()
+ * @see odp_packet_l2(), odp_packet_data()
  */
 uint8_t *odp_packet_addr(odp_packet_t pkt);
 
 /**
- * Get pointer to the start of the received frame
- *
- * The address of the packet buffer is not necessarily the same as the start
- * address of the received frame, e.g. an eth frame may be offset by 2 or 6
- * bytes to ensure 32 or 64-bit alignment of the IP header.
- * Use odp_packet_l2(pkt) to get the start address of a received valid eth frame
+ * Packet data address
  *
- * odp_packet_start() will always return a pointer to the start of the frame,
- * even if the frame is unrecognized and no valid L2 header could be found.
+ * Returns the current packet data address. When a packet is received from
+ * packet input, the data address points to the first byte of the packet.
  *
  * @param pkt  Packet handle
  *
- * @return  Pointer to the start of the received frame
+ * @return  Pointer to the packet data
  *
  * @see odp_packet_l2(), odp_packet_addr()
  */
-uint8_t *odp_packet_start(odp_packet_t pkt);
+uint8_t *odp_packet_data(odp_packet_t pkt);
 
 /**
  * Get pointer to the start of the L2 frame
@@ -162,7 +158,7 @@  uint8_t *odp_packet_start(odp_packet_t pkt);
  *
  * @return  Pointer to L2 header or NULL if not found
  *
- * @see odp_packet_addr(), odp_packet_start()
+ * @see odp_packet_addr(), odp_packet_data()
  */
 uint8_t *odp_packet_l2(odp_packet_t pkt);
 
diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-generic/odp_packet.c
index 6cd6183..435bc8b 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -61,7 +61,7 @@  uint8_t *odp_packet_addr(odp_packet_t pkt)
 	return odp_buffer_addr(odp_buffer_from_packet(pkt));
 }
 
-uint8_t *odp_packet_start(odp_packet_t pkt)
+uint8_t *odp_packet_data(odp_packet_t pkt)
 {
 	return odp_packet_addr(pkt) + odp_packet_hdr(pkt)->frame_offset;
 }
@@ -183,7 +183,7 @@  void odp_packet_parse(odp_packet_t pkt, size_t len, size_t frame_offset)
 	pkt_hdr->input_flags.l2 = 1;
 	pkt_hdr->l2_offset = frame_offset;
 
-	eth = (odph_ethhdr_t *)odp_packet_start(pkt);
+	eth = (odph_ethhdr_t *)odp_packet_data(pkt);
 	ethtype = odp_be_to_cpu_16(eth->type);
 	vlan = (odph_vlanhdr_t *)&eth->type;
 
@@ -372,8 +372,8 @@  int odp_packet_copy(odp_packet_t pkt_dst, odp_packet_t pkt_src)
 	memcpy(start_dst, start_src, len);
 
 	/* Copy frame payload */
-	start_dst = (uint8_t *)odp_packet_start(pkt_dst);
-	start_src = (uint8_t *)odp_packet_start(pkt_src);
+	start_dst = (uint8_t *)odp_packet_data(pkt_dst);
+	start_src = (uint8_t *)odp_packet_data(pkt_src);
 	len = pkt_hdr_src->frame_len;
 	memcpy(start_dst, start_src, len);