Re: [lng-odp] [PATCH API-NEXT v1] api: packet: data and segment length
Petri Savolainen(psavol) replied on github web page: include/odp/api/spec/packet.h line 17 @@ -401,30 +401,39 @@ uint32_t odp_packet_buf_len(odp_packet_t pkt); /** * Packet data pointer * - * Returns the current packet data pointer. When a packet is received - * from packet input, this points to the first byte of the received - * packet. Packet level offsets are calculated relative to this position. + * Returns pointer to the first byte of packet data. When packet is segmented, + * only a portion of packet data follows the pointer. When unsure, use e.g. + * odp_packet_seg_len() to check the data length following the pointer. Packet + * level offsets are calculated relative to this position. * - * User can adjust the data pointer with head_push/head_pull (does not modify - * segmentation) and add_data/rem_data calls (may modify segmentation). + * When a packet is received from packet input, this points to the first byte + * of the received packet. Pool configuration parameters may be used to ensure + * that the first packet segment contains all/most of the data relevant to the + * application. Comment: "Minimum number of packet data bytes that are stored in the ... " would be clearer with "... that can be stored ... ". Pool config ensures that packet has large enough first segment for storing seg_len bytes. Packet input documentation could state that packet input always fills the first segment fully. When application manipulates packet, various segment sizes may result. > JannePeltonen wrote > I was wondering if the above statement on pool configuration is accurate and > checked that pool configuration documentation says this: > > /** Minimum number of packet data bytes that are stored in the > * first segment of a packet. The maximum value is defined by > * pool capability pkt.max_seg_len. Use 0 for default. > */ > uint32_t seg_len; > > I think it is unclear what _exactly_ is meant by the above. For instance, > right after allocating a new packet, the first segment contains zero data > bytes regardless of the seg_len setting. I suppose the intent is that the > first segment contains the minimum of seg_len and odp_packet_len() data > bytes. But when does that condition hold? After receiving a packet from pktio > (yes)? After receiving a packet from a queue (dunno)? After extending or > truncating or otherwise manipulating the packet buffer (probably not)? > > I would suggest clarifying the documentation of the seg_len pool parameter. > This is related to this patch indirectly through the reference to the pool > configuration. >> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >> Segments are inherently implementation-dependent, which is why portable >> applications should avoid processing packets in terms of segments. >> Applications only need be aware that segments may exist, meaning that >> packets may not be contiguously addressable. This is why a `seg_len` is >> returned on the various routines that provide addressability to packets. >> `odp_packet_data()` is the exception, and this was done as an efficiency >> measure for applications looking at headers contained at the start of the >> packet that are known to be contiguous because of the `min_seg_len` pool >> specification. >>> Petri Savolainen(psavol) wrote: >>> Yes. I didn't want to introduce a new limitation to segment/reference >>> implementation here. This patch just tries to make it clear that packets >>> may be segmented. >>> >>> E.g. Bill's reference implementation resulted packets that had first >>> segment length 0 and data pointer pointed to the second segment. It passed >>> all validation tests. From application point of view, it does not matter >>> much if spec allows empty segments to be linked into packet, although it's >>> not very intuitive and should be avoided when possible. Balasubramanian Manoharan(bala-manoharan) wrote: In the entire documentation you have avoided the term "first segment" is it by choice? IMO we could refer this as first segment valid data bytes > Petri Savolainen(psavol) wrote: > seg_len / push_head / extend_head are mentioned above. Packet_offset is > not specialized for handling first N bytes of packet, so it's not > directly related to these ones. >> Petri Savolainen(psavol) wrote: >> packet_offset is for different purpose (access data on arbitrary >> offset), these calls are optimized for the common case (offset zero). >> Also odp_packet_seg_data_len(), the new data_seg_len(), >> odp_packet_l2_ptr(), odp_packet_l3_ptr() and odp_packet_l4_ptr() output >> seg len, but we don't need to list all possible ways to get it. It's >> enough that the reader understands that a packet may have segments and >> segment length is different thing than total packet length >>> Dmitry Eremin-Solenikov(lumag) w
Re: [lng-odp] [PATCH API-NEXT v1] api: packet: data and segment length
JannePeltonen replied on github web page: include/odp/api/spec/packet.h line 17 @@ -401,30 +401,39 @@ uint32_t odp_packet_buf_len(odp_packet_t pkt); /** * Packet data pointer * - * Returns the current packet data pointer. When a packet is received - * from packet input, this points to the first byte of the received - * packet. Packet level offsets are calculated relative to this position. + * Returns pointer to the first byte of packet data. When packet is segmented, + * only a portion of packet data follows the pointer. When unsure, use e.g. + * odp_packet_seg_len() to check the data length following the pointer. Packet + * level offsets are calculated relative to this position. * - * User can adjust the data pointer with head_push/head_pull (does not modify - * segmentation) and add_data/rem_data calls (may modify segmentation). + * When a packet is received from packet input, this points to the first byte + * of the received packet. Pool configuration parameters may be used to ensure + * that the first packet segment contains all/most of the data relevant to the + * application. Comment: I was wondering if the above statement on pool configuration is accurate and checked that pool configuration documentation says this: /** Minimum number of packet data bytes that are stored in the * first segment of a packet. The maximum value is defined by * pool capability pkt.max_seg_len. Use 0 for default. */ uint32_t seg_len; I think it is unclear what _exactly_ is meant by the above. For instance, right after allocating a new packet, the first segment contains zero data bytes regardless of the seg_len setting. I suppose the intent is that the first segment contains the minimum of seg_len and odp_packet_len() data bytes. But when does that condition hold? After receiving a packet from pktio (yes)? After receiving a packet from a queue (dunno)? After extending or truncating or otherwise manipulating the packet buffer (probably not)? I would suggest clarifying the documentation of the seg_len pool parameter. This is related to this patch indirectly through the reference to the pool configuration. > Bill Fischofer(Bill-Fischofer-Linaro) wrote: > Segments are inherently implementation-dependent, which is why portable > applications should avoid processing packets in terms of segments. > Applications only need be aware that segments may exist, meaning that packets > may not be contiguously addressable. This is why a `seg_len` is returned on > the various routines that provide addressability to packets. > `odp_packet_data()` is the exception, and this was done as an efficiency > measure for applications looking at headers contained at the start of the > packet that are known to be contiguous because of the `min_seg_len` pool > specification. >> Petri Savolainen(psavol) wrote: >> Yes. I didn't want to introduce a new limitation to segment/reference >> implementation here. This patch just tries to make it clear that packets may >> be segmented. >> >> E.g. Bill's reference implementation resulted packets that had first >> segment length 0 and data pointer pointed to the second segment. It passed >> all validation tests. From application point of view, it does not matter >> much if spec allows empty segments to be linked into packet, although it's >> not very intuitive and should be avoided when possible. >>> Balasubramanian Manoharan(bala-manoharan) wrote: >>> In the entire documentation you have avoided the term "first segment" is it >>> by choice? >>> IMO we could refer this as first segment valid data bytes Petri Savolainen(psavol) wrote: seg_len / push_head / extend_head are mentioned above. Packet_offset is not specialized for handling first N bytes of packet, so it's not directly related to these ones. > Petri Savolainen(psavol) wrote: > packet_offset is for different purpose (access data on arbitrary offset), > these calls are optimized for the common case (offset zero). Also > odp_packet_seg_data_len(), the new data_seg_len(), odp_packet_l2_ptr(), > odp_packet_l3_ptr() and odp_packet_l4_ptr() output seg len, but we don't > need to list all possible ways to get it. It's enough that the reader > understands that a packet may have segments and segment length is > different thing than total packet length >> Dmitry Eremin-Solenikov(lumag) wrote: >> And here too, please. >>> Dmitry Eremin-Solenikov(lumag) wrote: >>> odp_packet_seg_len() **or odp_packet_offset()**, if you don't mind. https://github.com/Linaro/odp/pull/497#discussion_r170524772 updated_at 2018-02-26 08:51:10
Re: [lng-odp] [PATCH API-NEXT v1] api: packet: data and segment length
Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page: include/odp/api/spec/packet.h line 39 @@ -401,30 +401,39 @@ uint32_t odp_packet_buf_len(odp_packet_t pkt); /** * Packet data pointer * - * Returns the current packet data pointer. When a packet is received - * from packet input, this points to the first byte of the received - * packet. Packet level offsets are calculated relative to this position. + * Returns pointer to the first byte of packet data. When packet is segmented, + * only a portion of packet data follows the pointer. When unsure, use e.g. + * odp_packet_seg_len() to check the data length following the pointer. Packet + * level offsets are calculated relative to this position. * - * User can adjust the data pointer with head_push/head_pull (does not modify - * segmentation) and add_data/rem_data calls (may modify segmentation). + * When a packet is received from packet input, this points to the first byte + * of the received packet. Pool configuration parameters may be used to ensure + * that the first packet segment contains all/most of the data relevant to the + * application. + * + * User can adjust the data pointer with e.g. push_head/pull_head (does not + * modify segmentation) and extend_head/trunc_head (may modify segmentation) + * calls. * * @param pkt Packet handle * * @return Pointer to the packet data * - * @see odp_packet_l2_ptr(), odp_packet_seg_len() + * @see odp_packet_seg_len(), odp_packet_push_head(), odp_packet_extend_head() */ void *odp_packet_data(odp_packet_t pkt); /** - * Packet segment data length + * Packet data length following the data pointer * - * Returns number of data bytes following the current data pointer - * (odp_packet_data()) location in the segment. + * Returns number of data bytes (in the segment) following the current data + * pointer position. When unsure, use this function to check how many bytes Comment: Segments are inherently implementation-dependent, which is why portable applications should avoid processing packets in terms of segments. Applications only need be aware that segments may exist, meaning that packets may not be contiguously addressable. This is why a `seg_len` is returned on the various routines that provide addressability to packets. `odp_packet_data()` is the exception, and this was done as an efficiency measure for applications looking at headers contained at the start of the packet that are known to be contiguous because of the `min_seg_len` pool specification. > Petri Savolainen(psavol) wrote: > Yes. I didn't want to introduce a new limitation to segment/reference > implementation here. This patch just tries to make it clear that packets may > be segmented. > > E.g. Bill's reference implementation resulted packets that had first segment > length 0 and data pointer pointed to the second segment. It passed all > validation tests. From application point of view, it does not matter much if > spec allows empty segments to be linked into packet, although it's not very > intuitive and should be avoided when possible. >> Balasubramanian Manoharan(bala-manoharan) wrote: >> In the entire documentation you have avoided the term "first segment" is it >> by choice? >> IMO we could refer this as first segment valid data bytes >>> Petri Savolainen(psavol) wrote: >>> seg_len / push_head / extend_head are mentioned above. Packet_offset is not >>> specialized for handling first N bytes of packet, so it's not directly >>> related to these ones. Petri Savolainen(psavol) wrote: packet_offset is for different purpose (access data on arbitrary offset), these calls are optimized for the common case (offset zero). Also odp_packet_seg_data_len(), the new data_seg_len(), odp_packet_l2_ptr(), odp_packet_l3_ptr() and odp_packet_l4_ptr() output seg len, but we don't need to list all possible ways to get it. It's enough that the reader understands that a packet may have segments and segment length is different thing than total packet length > Dmitry Eremin-Solenikov(lumag) wrote: > And here too, please. >> Dmitry Eremin-Solenikov(lumag) wrote: >> odp_packet_seg_len() **or odp_packet_offset()**, if you don't mind. https://github.com/Linaro/odp/pull/497#discussion_r170275071 updated_at 2018-02-23 15:06:52
Re: [lng-odp] [PATCH API-NEXT v1] api: packet: data and segment length
Petri Savolainen(psavol) replied on github web page: include/odp/api/spec/packet.h line 39 @@ -401,30 +401,39 @@ uint32_t odp_packet_buf_len(odp_packet_t pkt); /** * Packet data pointer * - * Returns the current packet data pointer. When a packet is received - * from packet input, this points to the first byte of the received - * packet. Packet level offsets are calculated relative to this position. + * Returns pointer to the first byte of packet data. When packet is segmented, + * only a portion of packet data follows the pointer. When unsure, use e.g. + * odp_packet_seg_len() to check the data length following the pointer. Packet + * level offsets are calculated relative to this position. * - * User can adjust the data pointer with head_push/head_pull (does not modify - * segmentation) and add_data/rem_data calls (may modify segmentation). + * When a packet is received from packet input, this points to the first byte + * of the received packet. Pool configuration parameters may be used to ensure + * that the first packet segment contains all/most of the data relevant to the + * application. + * + * User can adjust the data pointer with e.g. push_head/pull_head (does not + * modify segmentation) and extend_head/trunc_head (may modify segmentation) + * calls. * * @param pkt Packet handle * * @return Pointer to the packet data * - * @see odp_packet_l2_ptr(), odp_packet_seg_len() + * @see odp_packet_seg_len(), odp_packet_push_head(), odp_packet_extend_head() */ void *odp_packet_data(odp_packet_t pkt); /** - * Packet segment data length + * Packet data length following the data pointer * - * Returns number of data bytes following the current data pointer - * (odp_packet_data()) location in the segment. + * Returns number of data bytes (in the segment) following the current data + * pointer position. When unsure, use this function to check how many bytes Comment: Yes. I didn't want to introduce a new limitation to segment/reference implementation here. This patch just tries to make it clear that packets may be segmented. E.g. Bill's reference implementation resulted packets that had first segment length 0 and data pointer pointed to the second segment. It passed all validation tests. From application point of view, it does not matter much if spec allows empty segments to be linked into packet, although it's not very intuitive and should be avoided when possible. > Balasubramanian Manoharan(bala-manoharan) wrote: > In the entire documentation you have avoided the term "first segment" is it > by choice? > IMO we could refer this as first segment valid data bytes >> Petri Savolainen(psavol) wrote: >> seg_len / push_head / extend_head are mentioned above. Packet_offset is not >> specialized for handling first N bytes of packet, so it's not directly >> related to these ones. >>> Petri Savolainen(psavol) wrote: >>> packet_offset is for different purpose (access data on arbitrary offset), >>> these calls are optimized for the common case (offset zero). Also >>> odp_packet_seg_data_len(), the new data_seg_len(), odp_packet_l2_ptr(), >>> odp_packet_l3_ptr() and odp_packet_l4_ptr() output seg len, but we don't >>> need to list all possible ways to get it. It's enough that the reader >>> understands that a packet may have segments and segment length is different >>> thing than total packet length Dmitry Eremin-Solenikov(lumag) wrote: And here too, please. > Dmitry Eremin-Solenikov(lumag) wrote: > odp_packet_seg_len() **or odp_packet_offset()**, if you don't mind. https://github.com/Linaro/odp/pull/497#discussion_r170220664 updated_at 2018-02-23 10:45:55
Re: [lng-odp] [PATCH API-NEXT v1] api: packet: data and segment length
Balasubramanian Manoharan(bala-manoharan) replied on github web page: include/odp/api/spec/packet.h line 39 @@ -401,30 +401,39 @@ uint32_t odp_packet_buf_len(odp_packet_t pkt); /** * Packet data pointer * - * Returns the current packet data pointer. When a packet is received - * from packet input, this points to the first byte of the received - * packet. Packet level offsets are calculated relative to this position. + * Returns pointer to the first byte of packet data. When packet is segmented, + * only a portion of packet data follows the pointer. When unsure, use e.g. + * odp_packet_seg_len() to check the data length following the pointer. Packet + * level offsets are calculated relative to this position. * - * User can adjust the data pointer with head_push/head_pull (does not modify - * segmentation) and add_data/rem_data calls (may modify segmentation). + * When a packet is received from packet input, this points to the first byte + * of the received packet. Pool configuration parameters may be used to ensure + * that the first packet segment contains all/most of the data relevant to the + * application. + * + * User can adjust the data pointer with e.g. push_head/pull_head (does not + * modify segmentation) and extend_head/trunc_head (may modify segmentation) + * calls. * * @param pkt Packet handle * * @return Pointer to the packet data * - * @see odp_packet_l2_ptr(), odp_packet_seg_len() + * @see odp_packet_seg_len(), odp_packet_push_head(), odp_packet_extend_head() */ void *odp_packet_data(odp_packet_t pkt); /** - * Packet segment data length + * Packet data length following the data pointer * - * Returns number of data bytes following the current data pointer - * (odp_packet_data()) location in the segment. + * Returns number of data bytes (in the segment) following the current data + * pointer position. When unsure, use this function to check how many bytes Comment: In the entire documentation you have avoided the term "first segment" is it by choice? IMO we could refer this as first segment valid data bytes > Petri Savolainen(psavol) wrote: > seg_len / push_head / extend_head are mentioned above. Packet_offset is not > specialized for handling first N bytes of packet, so it's not directly > related to these ones. >> Petri Savolainen(psavol) wrote: >> packet_offset is for different purpose (access data on arbitrary offset), >> these calls are optimized for the common case (offset zero). Also >> odp_packet_seg_data_len(), the new data_seg_len(), odp_packet_l2_ptr(), >> odp_packet_l3_ptr() and odp_packet_l4_ptr() output seg len, but we don't >> need to list all possible ways to get it. It's enough that the reader >> understands that a packet may have segments and segment length is different >> thing than total packet length >>> Dmitry Eremin-Solenikov(lumag) wrote: >>> And here too, please. Dmitry Eremin-Solenikov(lumag) wrote: odp_packet_seg_len() **or odp_packet_offset()**, if you don't mind. https://github.com/Linaro/odp/pull/497#discussion_r170204670 updated_at 2018-02-23 09:36:37
Re: [lng-odp] [PATCH API-NEXT v1] api: packet: data and segment length
Petri Savolainen(psavol) replied on github web page: include/odp/api/spec/packet.h line 28 @@ -401,30 +401,39 @@ uint32_t odp_packet_buf_len(odp_packet_t pkt); /** * Packet data pointer * - * Returns the current packet data pointer. When a packet is received - * from packet input, this points to the first byte of the received - * packet. Packet level offsets are calculated relative to this position. + * Returns pointer to the first byte of packet data. When packet is segmented, + * only a portion of packet data follows the pointer. When unsure, use e.g. + * odp_packet_seg_len() to check the data length following the pointer. Packet + * level offsets are calculated relative to this position. * - * User can adjust the data pointer with head_push/head_pull (does not modify - * segmentation) and add_data/rem_data calls (may modify segmentation). + * When a packet is received from packet input, this points to the first byte + * of the received packet. Pool configuration parameters may be used to ensure + * that the first packet segment contains all/most of the data relevant to the + * application. + * + * User can adjust the data pointer with e.g. push_head/pull_head (does not + * modify segmentation) and extend_head/trunc_head (may modify segmentation) + * calls. * * @param pkt Packet handle * * @return Pointer to the packet data * - * @see odp_packet_l2_ptr(), odp_packet_seg_len() + * @see odp_packet_seg_len(), odp_packet_push_head(), odp_packet_extend_head() Comment: seg_len / push_head / extend_head are mentioned above. Packet_offset is not specialized for handling first N bytes of packet, so it's not directly related to these ones. > Petri Savolainen(psavol) wrote: > packet_offset is for different purpose (access data on arbitrary offset), > these calls are optimized for the common case (offset zero). Also > odp_packet_seg_data_len(), the new data_seg_len(), odp_packet_l2_ptr(), > odp_packet_l3_ptr() and odp_packet_l4_ptr() output seg len, but we don't > need to list all possible ways to get it. It's enough that the reader > understands that a packet may have segments and segment length is different > thing than total packet length >> Dmitry Eremin-Solenikov(lumag) wrote: >> And here too, please. >>> Dmitry Eremin-Solenikov(lumag) wrote: >>> odp_packet_seg_len() **or odp_packet_offset()**, if you don't mind. https://github.com/Linaro/odp/pull/497#discussion_r170183024 updated_at 2018-02-23 07:36:58
Re: [lng-odp] [PATCH API-NEXT v1] api: packet: data and segment length
Petri Savolainen(psavol) replied on github web page: include/odp/api/spec/packet.h line 9 @@ -401,30 +401,39 @@ uint32_t odp_packet_buf_len(odp_packet_t pkt); /** * Packet data pointer * - * Returns the current packet data pointer. When a packet is received - * from packet input, this points to the first byte of the received - * packet. Packet level offsets are calculated relative to this position. + * Returns pointer to the first byte of packet data. When packet is segmented, + * only a portion of packet data follows the pointer. When unsure, use e.g. + * odp_packet_seg_len() to check the data length following the pointer. Packet Comment: packet_offset is for different purpose (access data on arbitrary offset), these calls are optimized for the common case (offset zero). Also odp_packet_seg_data_len(), the new data_seg_len(), odp_packet_l2_ptr(), odp_packet_l3_ptr() and odp_packet_l4_ptr() output seg len, but we don't need to list all possible ways to get it. It's enough that the reader understands that a packet may have segments and segment length is different thing than total packet length > Dmitry Eremin-Solenikov(lumag) wrote: > And here too, please. >> Dmitry Eremin-Solenikov(lumag) wrote: >> odp_packet_seg_len() **or odp_packet_offset()**, if you don't mind. https://github.com/Linaro/odp/pull/497#discussion_r170182482 updated_at 2018-02-23 07:33:19
Re: [lng-odp] [PATCH API-NEXT v1] api: packet: data and segment length
Dmitry Eremin-Solenikov(lumag) replied on github web page: include/odp/api/spec/packet.h line 28 @@ -401,30 +401,39 @@ uint32_t odp_packet_buf_len(odp_packet_t pkt); /** * Packet data pointer * - * Returns the current packet data pointer. When a packet is received - * from packet input, this points to the first byte of the received - * packet. Packet level offsets are calculated relative to this position. + * Returns pointer to the first byte of packet data. When packet is segmented, + * only a portion of packet data follows the pointer. When unsure, use e.g. + * odp_packet_seg_len() to check the data length following the pointer. Packet + * level offsets are calculated relative to this position. * - * User can adjust the data pointer with head_push/head_pull (does not modify - * segmentation) and add_data/rem_data calls (may modify segmentation). + * When a packet is received from packet input, this points to the first byte + * of the received packet. Pool configuration parameters may be used to ensure + * that the first packet segment contains all/most of the data relevant to the + * application. + * + * User can adjust the data pointer with e.g. push_head/pull_head (does not + * modify segmentation) and extend_head/trunc_head (may modify segmentation) + * calls. * * @param pkt Packet handle * * @return Pointer to the packet data * - * @see odp_packet_l2_ptr(), odp_packet_seg_len() + * @see odp_packet_seg_len(), odp_packet_push_head(), odp_packet_extend_head() Comment: And here too, please. > Dmitry Eremin-Solenikov(lumag) wrote: > odp_packet_seg_len() **or odp_packet_offset()**, if you don't mind. https://github.com/Linaro/odp/pull/497#discussion_r170095913 updated_at 2018-02-22 21:18:09
Re: [lng-odp] [PATCH API-NEXT v1] api: packet: data and segment length
Dmitry Eremin-Solenikov(lumag) replied on github web page: include/odp/api/spec/packet.h line 9 @@ -401,30 +401,39 @@ uint32_t odp_packet_buf_len(odp_packet_t pkt); /** * Packet data pointer * - * Returns the current packet data pointer. When a packet is received - * from packet input, this points to the first byte of the received - * packet. Packet level offsets are calculated relative to this position. + * Returns pointer to the first byte of packet data. When packet is segmented, + * only a portion of packet data follows the pointer. When unsure, use e.g. + * odp_packet_seg_len() to check the data length following the pointer. Packet Comment: odp_packet_seg_len() **or odp_packet_offset()**, if you don't mind. https://github.com/Linaro/odp/pull/497#discussion_r170095823 updated_at 2018-02-22 21:17:40
