Re: [lng-odp] [PATCH 2/3] validation: buffer: fix for the use of cached return value of odp_packet_last_seg
On Wed, Jan 28, 2015 at 12:02:45PM -0500, Mike Holmes wrote: Any resolution, I know there have been discussions on going but I need to know if this is in 0.10.0 According to Petri, Application expects to reuse the seg handle after push/pull operations. so, No need to change the existing specification hence this patch is not applicable. validation: buffer: check the return value of odp_packet_l?_offset_set can be included in 0.10 On 27 January 2015 at 09:12, Mike Holmes mike.hol...@linaro.org wrote: Can we resolve this for 0.10.0 On 25 January 2015 at 22:40, Jerin Jacob jerin.ja...@caviumnetworks.com wrote: On Sun, Jan 25, 2015 at 03:01:24PM +0200, Taras Kondratiuk wrote: On 01/24/2015 08:06 PM, Bill Fischofer wrote: The issue is not whether the number of segments is changed but whether the pkt handle is changed. APIs that potentially substitute a new handle must return a handle. If a handle is not returned then the input handle is unchanged, however behavior of subsequent APIs against that handle may of course change. To take a trivial example, odp_packet_len() will obviously change following a push/pull even though the same handle is used. I agree with Jerin that we really need to take care to not overspecify behavior on ODP APIs. API observable effects should be minimally specified to allow implementations latitude in how best to map the required behavior to their platform. In the case of push/pull the purpose is simply to add or subtract bytes at the start or end of a packet. That's the essential function of these APIs and is the only thing that should be specified as required behavior. In the case of segmentation there are two philosophies one can adopt. The first is that the application desires and needs to have explicit control over packet segmentation. The second is that any packet segmentation is the responsibility of the implementation and the application need only be aware that packets may be segmented and take that into account in its design, realizing that segments are the units of contiguous addressability. The former is a very software-centric view which provides limited opportunity for implementations to optimize performance, and may in fact be impossible to implement efficiently on some platforms. The latter requires that existing software-centric applications may need some more redesign to better adapt to ODP. But the benefits of such adaptation is cleaner portability across a wider range of platforms with optimal performance on each, and that's really the goal of ODP. I agree on most of these items and I do understand Jerin's restrictions, but my main point: current test does follow specification. If it can't be implemented efficiently on a platform, then specification should be changed first. OK, How about following change in the specification ? diff --git a/platform/linux-generic/include/api/odp_packet.h b/platform/linux-generic/include/api/odp_packet.h index 920a593..e418e42 100644 --- a/platform/linux-generic/include/api/odp_packet.h +++ b/platform/linux-generic/include/api/odp_packet.h @@ -244,7 +244,7 @@ void *odp_packet_tail(odp_packet_t pkt); * headroom -= len * data -= len * - * Operation does not modify packet segmentation or move data. Handles and + * Operation does not modify packet segmentation or move data. pkt handle and * pointers remain valid. User is responsible to update packet metadata * offsets when needed. * @@ -272,7 +272,7 @@ void *odp_packet_push_head(odp_packet_t pkt, uint32_t len); * headroom += len * data += len * - * Operation does not modify packet segmentation or move data. Handles and + * Operation does not modify packet segmentation or move data. pkt handle and * pointers remain valid. User is responsible to update packet metadata * offsets when needed. * @@ -302,7 +302,7 @@ void *odp_packet_pull_head(odp_packet_t pkt, uint32_t len); * tail += len * tailroom -= len * - * Operation does not modify packet segmentation or move data. Handles, + * Operation does not modify packet segmentation or move data. pkt handle, * pointers and offsets remain valid. * * @param pkt Packet handle @@ -331,7 +331,7 @@ void *odp_packet_push_tail(odp_packet_t pkt, uint32_t len); * tail -= len * tailroom += len * - * Operation does not modify packet segmentation or move data. Handles and + * Operation does not modify packet segmentation or move data. pkt handle and * pointers remain valid. User is responsible to update packet metadata * offsets when needed. * ___ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp --
Re: [lng-odp] [PATCH 2/3] validation: buffer: fix for the use of cached return value of odp_packet_last_seg
Any resolution, I know there have been discussions on going but I need to know if this is in 0.10.0 On 27 January 2015 at 09:12, Mike Holmes mike.hol...@linaro.org wrote: Can we resolve this for 0.10.0 On 25 January 2015 at 22:40, Jerin Jacob jerin.ja...@caviumnetworks.com wrote: On Sun, Jan 25, 2015 at 03:01:24PM +0200, Taras Kondratiuk wrote: On 01/24/2015 08:06 PM, Bill Fischofer wrote: The issue is not whether the number of segments is changed but whether the pkt handle is changed. APIs that potentially substitute a new handle must return a handle. If a handle is not returned then the input handle is unchanged, however behavior of subsequent APIs against that handle may of course change. To take a trivial example, odp_packet_len() will obviously change following a push/pull even though the same handle is used. I agree with Jerin that we really need to take care to not overspecify behavior on ODP APIs. API observable effects should be minimally specified to allow implementations latitude in how best to map the required behavior to their platform. In the case of push/pull the purpose is simply to add or subtract bytes at the start or end of a packet. That's the essential function of these APIs and is the only thing that should be specified as required behavior. In the case of segmentation there are two philosophies one can adopt. The first is that the application desires and needs to have explicit control over packet segmentation. The second is that any packet segmentation is the responsibility of the implementation and the application need only be aware that packets may be segmented and take that into account in its design, realizing that segments are the units of contiguous addressability. The former is a very software-centric view which provides limited opportunity for implementations to optimize performance, and may in fact be impossible to implement efficiently on some platforms. The latter requires that existing software-centric applications may need some more redesign to better adapt to ODP. But the benefits of such adaptation is cleaner portability across a wider range of platforms with optimal performance on each, and that's really the goal of ODP. I agree on most of these items and I do understand Jerin's restrictions, but my main point: current test does follow specification. If it can't be implemented efficiently on a platform, then specification should be changed first. OK, How about following change in the specification ? diff --git a/platform/linux-generic/include/api/odp_packet.h b/platform/linux-generic/include/api/odp_packet.h index 920a593..e418e42 100644 --- a/platform/linux-generic/include/api/odp_packet.h +++ b/platform/linux-generic/include/api/odp_packet.h @@ -244,7 +244,7 @@ void *odp_packet_tail(odp_packet_t pkt); * headroom -= len * data -= len * - * Operation does not modify packet segmentation or move data. Handles and + * Operation does not modify packet segmentation or move data. pkt handle and * pointers remain valid. User is responsible to update packet metadata * offsets when needed. * @@ -272,7 +272,7 @@ void *odp_packet_push_head(odp_packet_t pkt, uint32_t len); * headroom += len * data += len * - * Operation does not modify packet segmentation or move data. Handles and + * Operation does not modify packet segmentation or move data. pkt handle and * pointers remain valid. User is responsible to update packet metadata * offsets when needed. * @@ -302,7 +302,7 @@ void *odp_packet_pull_head(odp_packet_t pkt, uint32_t len); * tail += len * tailroom -= len * - * Operation does not modify packet segmentation or move data. Handles, + * Operation does not modify packet segmentation or move data. pkt handle, * pointers and offsets remain valid. * * @param pkt Packet handle @@ -331,7 +331,7 @@ void *odp_packet_push_tail(odp_packet_t pkt, uint32_t len); * tail -= len * tailroom += len * - * Operation does not modify packet segmentation or move data. Handles and + * Operation does not modify packet segmentation or move data. pkt handle and * pointers remain valid. User is responsible to update packet metadata * offsets when needed. * ___ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp -- *Mike Holmes* Linaro Sr Technical Manager LNG - ODP -- *Mike Holmes* Linaro Sr Technical Manager LNG - ODP ___ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH 2/3] validation: buffer: fix for the use of cached return value of odp_packet_last_seg
Can we resolve this for 0.10.0 On 25 January 2015 at 22:40, Jerin Jacob jerin.ja...@caviumnetworks.com wrote: On Sun, Jan 25, 2015 at 03:01:24PM +0200, Taras Kondratiuk wrote: On 01/24/2015 08:06 PM, Bill Fischofer wrote: The issue is not whether the number of segments is changed but whether the pkt handle is changed. APIs that potentially substitute a new handle must return a handle. If a handle is not returned then the input handle is unchanged, however behavior of subsequent APIs against that handle may of course change. To take a trivial example, odp_packet_len() will obviously change following a push/pull even though the same handle is used. I agree with Jerin that we really need to take care to not overspecify behavior on ODP APIs. API observable effects should be minimally specified to allow implementations latitude in how best to map the required behavior to their platform. In the case of push/pull the purpose is simply to add or subtract bytes at the start or end of a packet. That's the essential function of these APIs and is the only thing that should be specified as required behavior. In the case of segmentation there are two philosophies one can adopt. The first is that the application desires and needs to have explicit control over packet segmentation. The second is that any packet segmentation is the responsibility of the implementation and the application need only be aware that packets may be segmented and take that into account in its design, realizing that segments are the units of contiguous addressability. The former is a very software-centric view which provides limited opportunity for implementations to optimize performance, and may in fact be impossible to implement efficiently on some platforms. The latter requires that existing software-centric applications may need some more redesign to better adapt to ODP. But the benefits of such adaptation is cleaner portability across a wider range of platforms with optimal performance on each, and that's really the goal of ODP. I agree on most of these items and I do understand Jerin's restrictions, but my main point: current test does follow specification. If it can't be implemented efficiently on a platform, then specification should be changed first. OK, How about following change in the specification ? diff --git a/platform/linux-generic/include/api/odp_packet.h b/platform/linux-generic/include/api/odp_packet.h index 920a593..e418e42 100644 --- a/platform/linux-generic/include/api/odp_packet.h +++ b/platform/linux-generic/include/api/odp_packet.h @@ -244,7 +244,7 @@ void *odp_packet_tail(odp_packet_t pkt); * headroom -= len * data -= len * - * Operation does not modify packet segmentation or move data. Handles and + * Operation does not modify packet segmentation or move data. pkt handle and * pointers remain valid. User is responsible to update packet metadata * offsets when needed. * @@ -272,7 +272,7 @@ void *odp_packet_push_head(odp_packet_t pkt, uint32_t len); * headroom += len * data += len * - * Operation does not modify packet segmentation or move data. Handles and + * Operation does not modify packet segmentation or move data. pkt handle and * pointers remain valid. User is responsible to update packet metadata * offsets when needed. * @@ -302,7 +302,7 @@ void *odp_packet_pull_head(odp_packet_t pkt, uint32_t len); * tail += len * tailroom -= len * - * Operation does not modify packet segmentation or move data. Handles, + * Operation does not modify packet segmentation or move data. pkt handle, * pointers and offsets remain valid. * * @param pkt Packet handle @@ -331,7 +331,7 @@ void *odp_packet_push_tail(odp_packet_t pkt, uint32_t len); * tail -= len * tailroom += len * - * Operation does not modify packet segmentation or move data. Handles and + * Operation does not modify packet segmentation or move data. pkt handle and * pointers remain valid. User is responsible to update packet metadata * offsets when needed. * ___ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp -- *Mike Holmes* Linaro Sr Technical Manager LNG - ODP ___ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH 2/3] validation: buffer: fix for the use of cached return value of odp_packet_last_seg
On Sun, Jan 25, 2015 at 03:01:24PM +0200, Taras Kondratiuk wrote: On 01/24/2015 08:06 PM, Bill Fischofer wrote: The issue is not whether the number of segments is changed but whether the pkt handle is changed. APIs that potentially substitute a new handle must return a handle. If a handle is not returned then the input handle is unchanged, however behavior of subsequent APIs against that handle may of course change. To take a trivial example, odp_packet_len() will obviously change following a push/pull even though the same handle is used. I agree with Jerin that we really need to take care to not overspecify behavior on ODP APIs. API observable effects should be minimally specified to allow implementations latitude in how best to map the required behavior to their platform. In the case of push/pull the purpose is simply to add or subtract bytes at the start or end of a packet. That's the essential function of these APIs and is the only thing that should be specified as required behavior. In the case of segmentation there are two philosophies one can adopt. The first is that the application desires and needs to have explicit control over packet segmentation. The second is that any packet segmentation is the responsibility of the implementation and the application need only be aware that packets may be segmented and take that into account in its design, realizing that segments are the units of contiguous addressability. The former is a very software-centric view which provides limited opportunity for implementations to optimize performance, and may in fact be impossible to implement efficiently on some platforms. The latter requires that existing software-centric applications may need some more redesign to better adapt to ODP. But the benefits of such adaptation is cleaner portability across a wider range of platforms with optimal performance on each, and that's really the goal of ODP. I agree on most of these items and I do understand Jerin's restrictions, but my main point: current test does follow specification. If it can't be implemented efficiently on a platform, then specification should be changed first. OK, How about following change in the specification ? diff --git a/platform/linux-generic/include/api/odp_packet.h b/platform/linux-generic/include/api/odp_packet.h index 920a593..e418e42 100644 --- a/platform/linux-generic/include/api/odp_packet.h +++ b/platform/linux-generic/include/api/odp_packet.h @@ -244,7 +244,7 @@ void *odp_packet_tail(odp_packet_t pkt); * headroom -= len * data -= len * - * Operation does not modify packet segmentation or move data. Handles and + * Operation does not modify packet segmentation or move data. pkt handle and * pointers remain valid. User is responsible to update packet metadata * offsets when needed. * @@ -272,7 +272,7 @@ void *odp_packet_push_head(odp_packet_t pkt, uint32_t len); * headroom += len * data += len * - * Operation does not modify packet segmentation or move data. Handles and + * Operation does not modify packet segmentation or move data. pkt handle and * pointers remain valid. User is responsible to update packet metadata * offsets when needed. * @@ -302,7 +302,7 @@ void *odp_packet_pull_head(odp_packet_t pkt, uint32_t len); * tail += len * tailroom -= len * - * Operation does not modify packet segmentation or move data. Handles, + * Operation does not modify packet segmentation or move data. pkt handle, * pointers and offsets remain valid. * * @param pkt Packet handle @@ -331,7 +331,7 @@ void *odp_packet_push_tail(odp_packet_t pkt, uint32_t len); * tail -= len * tailroom += len * - * Operation does not modify packet segmentation or move data. Handles and + * Operation does not modify packet segmentation or move data. pkt handle and * pointers remain valid. User is responsible to update packet metadata * offsets when needed. * ___ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH 2/3] validation: buffer: fix for the use of cached return value of odp_packet_last_seg
On 01/24/2015 08:06 PM, Bill Fischofer wrote: The issue is not whether the number of segments is changed but whether the pkt handle is changed. APIs that potentially substitute a new handle must return a handle. If a handle is not returned then the input handle is unchanged, however behavior of subsequent APIs against that handle may of course change. To take a trivial example, odp_packet_len() will obviously change following a push/pull even though the same handle is used. I agree with Jerin that we really need to take care to not overspecify behavior on ODP APIs. API observable effects should be minimally specified to allow implementations latitude in how best to map the required behavior to their platform. In the case of push/pull the purpose is simply to add or subtract bytes at the start or end of a packet. That's the essential function of these APIs and is the only thing that should be specified as required behavior. In the case of segmentation there are two philosophies one can adopt. The first is that the application desires and needs to have explicit control over packet segmentation. The second is that any packet segmentation is the responsibility of the implementation and the application need only be aware that packets may be segmented and take that into account in its design, realizing that segments are the units of contiguous addressability. The former is a very software-centric view which provides limited opportunity for implementations to optimize performance, and may in fact be impossible to implement efficiently on some platforms. The latter requires that existing software-centric applications may need some more redesign to better adapt to ODP. But the benefits of such adaptation is cleaner portability across a wider range of platforms with optimal performance on each, and that's really the goal of ODP. I agree on most of these items and I do understand Jerin's restrictions, but my main point: current test does follow specification. If it can't be implemented efficiently on a platform, then specification should be changed first. ___ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH 2/3] validation: buffer: fix for the use of cached return value of odp_packet_last_seg
The issue is not whether the number of segments is changed but whether the pkt handle is changed. APIs that potentially substitute a new handle must return a handle. If a handle is not returned then the input handle is unchanged, however behavior of subsequent APIs against that handle may of course change. To take a trivial example, odp_packet_len() will obviously change following a push/pull even though the same handle is used. I agree with Jerin that we really need to take care to not overspecify behavior on ODP APIs. API observable effects should be minimally specified to allow implementations latitude in how best to map the required behavior to their platform. In the case of push/pull the purpose is simply to add or subtract bytes at the start or end of a packet. That's the essential function of these APIs and is the only thing that should be specified as required behavior. In the case of segmentation there are two philosophies one can adopt. The first is that the application desires and needs to have explicit control over packet segmentation. The second is that any packet segmentation is the responsibility of the implementation and the application need only be aware that packets may be segmented and take that into account in its design, realizing that segments are the units of contiguous addressability. The former is a very software-centric view which provides limited opportunity for implementations to optimize performance, and may in fact be impossible to implement efficiently on some platforms. The latter requires that existing software-centric applications may need some more redesign to better adapt to ODP. But the benefits of such adaptation is cleaner portability across a wider range of platforms with optimal performance on each, and that's really the goal of ODP. On Sat, Jan 24, 2015 at 6:23 AM, Taras Kondratiuk taras.kondrat...@linaro.org wrote: On 01/19/2015 09:17 AM, Jerin Jacob wrote: On Sun, Jan 18, 2015 at 12:22:12AM +0200, Taras Kondratiuk wrote: On 01/17/2015 01:29 PM, Jerin Jacob wrote: odp_packet_seg_t is an opaque type, based on the implementation, the return value of odp_packet_last_seg can be changed after headroom/tailroom push/pull operation. No. By definition headroom/tailroom push/pull operations don't change segmentation. So the last segment must remain the same. For cavium, odp_packet_seg_t pretty much defined by the hardware spec (its next seg data addr and data size of the next segment) so obviously size of the segment will change after the tailroom push/pull operation. If odp specification demands for same seg handle value then I need to map it as pointer which points to the value. But its adds additional overhead for no good and even if we make it as pointer, it will fail in case of odp_packet_del_data. IMO, enforcing applications to not cache the odp_packet_seg_t for headroom/tailroom push/pull and odp_packet_add/del_data will give freedom to implementation to map best odp_packet_seg_t for the hardware. While push/pull can't change segmentation odp_packet_add/del_data API *can* do this. They explicitly return a new odp_packet_t handle. To allow push/pull operations to change segmentation they also should have a way to return a new packet handle, because data can be copied to another packet. Anyway if you want to do these change then specification should be updated first. ___ 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
Re: [lng-odp] [PATCH 2/3] validation: buffer: fix for the use of cached return value of odp_packet_last_seg
On 01/19/2015 12:18 PM, Ola Liljedahl wrote: On 17 January 2015 at 23:22, Taras Kondratiuk taras.kondrat...@linaro.org wrote: On 01/17/2015 01:29 PM, Jerin Jacob wrote: odp_packet_seg_t is an opaque type, based on the implementation, the return value of odp_packet_last_seg can be changed after headroom/tailroom push/pull operation. No. By definition headroom/tailroom push/pull operations don't change segmentation. So the last segment must remain the same. Don't we make segmentation visible to allow ODP implementations to use non-consecutive buffers to implement the buffer or packet the application is working with? An implementation might need to add another segment for a push operation e.g. if there is not space enough in the current head or tail segment. And it might be useful for ODP implementations to remove unused segments (for pull operations), some HW might not like to operate on empty segments (or indeed any segment with a small number of bytes of data). So I can't understand how ODP can define that push and pull operations shall *not* affect the underlying segmentation of the packet. According a current spec these operation do not modify segmentation for application to able to cache handles and/or pointers: Operation does not modify packet segmentation or move data. Handles and pointers remain valid. User is responsible to update packet metadata offsets when needed. There are more heavyweight functions which can be used if there is no enough headroom/tailroom: odp_packet_add_data()/odp_packet_rem_data(). ___ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH 2/3] validation: buffer: fix for the use of cached return value of odp_packet_last_seg
Bumping to 0.10.0 waiting on resolution of Taras and Olas comments. On 20 January 2015 at 17:56, Mike Holmes mike.hol...@linaro.org wrote: What is the state of this patch, we have a reviewed by but we also have comments - it is slated for 0.9 On 19 January 2015 at 05:18, Ola Liljedahl ola.liljed...@linaro.org wrote: On 17 January 2015 at 23:22, Taras Kondratiuk taras.kondrat...@linaro.org wrote: On 01/17/2015 01:29 PM, Jerin Jacob wrote: odp_packet_seg_t is an opaque type, based on the implementation, the return value of odp_packet_last_seg can be changed after headroom/tailroom push/pull operation. No. By definition headroom/tailroom push/pull operations don't change segmentation. So the last segment must remain the same. Don't we make segmentation visible to allow ODP implementations to use non-consecutive buffers to implement the buffer or packet the application is working with? An implementation might need to add another segment for a push operation e.g. if there is not space enough in the current head or tail segment. And it might be useful for ODP implementations to remove unused segments (for pull operations), some HW might not like to operate on empty segments (or indeed any segment with a small number of bytes of data). So I can't understand how ODP can define that push and pull operations shall *not* affect the underlying segmentation of the packet. Signed-off-by: Jerin Jacob jerin.ja...@caviumnetworks.com --- test/validation/buffer/odp_packet_test.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/validation/buffer/odp_packet_test.c b/test/validation/buffer/odp_packet_test.c index b6fa028..7c2b169 100644 --- a/test/validation/buffer/odp_packet_test.c +++ b/test/validation/buffer/odp_packet_test.c @@ -289,6 +289,9 @@ static void _verify_tailroom_shift(odp_packet_t pkt, tail = odp_packet_pull_tail(pkt, -shift); } + seg = odp_packet_last_seg(pkt); + CU_ASSERT(seg != ODP_SEGMENT_INVALID); + CU_ASSERT(tail != NULL); CU_ASSERT(odp_packet_seg_data_len(pkt, seg) == seg_data_len + shift); CU_ASSERT(odp_packet_len(pkt) == pkt_data_len + shift); -- Taras Kondratiuk ___ 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 -- *Mike Holmes* Linaro Sr Technical Manager LNG - ODP -- *Mike Holmes* Linaro Sr Technical Manager LNG - ODP ___ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH 2/3] validation: buffer: fix for the use of cached return value of odp_packet_last_seg
What is the state of this patch, we have a reviewed by but we also have comments - it is slated for 0.9 On 19 January 2015 at 05:18, Ola Liljedahl ola.liljed...@linaro.org wrote: On 17 January 2015 at 23:22, Taras Kondratiuk taras.kondrat...@linaro.org wrote: On 01/17/2015 01:29 PM, Jerin Jacob wrote: odp_packet_seg_t is an opaque type, based on the implementation, the return value of odp_packet_last_seg can be changed after headroom/tailroom push/pull operation. No. By definition headroom/tailroom push/pull operations don't change segmentation. So the last segment must remain the same. Don't we make segmentation visible to allow ODP implementations to use non-consecutive buffers to implement the buffer or packet the application is working with? An implementation might need to add another segment for a push operation e.g. if there is not space enough in the current head or tail segment. And it might be useful for ODP implementations to remove unused segments (for pull operations), some HW might not like to operate on empty segments (or indeed any segment with a small number of bytes of data). So I can't understand how ODP can define that push and pull operations shall *not* affect the underlying segmentation of the packet. Signed-off-by: Jerin Jacob jerin.ja...@caviumnetworks.com --- test/validation/buffer/odp_packet_test.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/validation/buffer/odp_packet_test.c b/test/validation/buffer/odp_packet_test.c index b6fa028..7c2b169 100644 --- a/test/validation/buffer/odp_packet_test.c +++ b/test/validation/buffer/odp_packet_test.c @@ -289,6 +289,9 @@ static void _verify_tailroom_shift(odp_packet_t pkt, tail = odp_packet_pull_tail(pkt, -shift); } + seg = odp_packet_last_seg(pkt); + CU_ASSERT(seg != ODP_SEGMENT_INVALID); + CU_ASSERT(tail != NULL); CU_ASSERT(odp_packet_seg_data_len(pkt, seg) == seg_data_len + shift); CU_ASSERT(odp_packet_len(pkt) == pkt_data_len + shift); -- Taras Kondratiuk ___ 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 -- *Mike Holmes* Linaro Sr Technical Manager LNG - ODP ___ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH 2/3] validation: buffer: fix for the use of cached return value of odp_packet_last_seg
On 17 January 2015 at 23:22, Taras Kondratiuk taras.kondrat...@linaro.org wrote: On 01/17/2015 01:29 PM, Jerin Jacob wrote: odp_packet_seg_t is an opaque type, based on the implementation, the return value of odp_packet_last_seg can be changed after headroom/tailroom push/pull operation. No. By definition headroom/tailroom push/pull operations don't change segmentation. So the last segment must remain the same. Don't we make segmentation visible to allow ODP implementations to use non-consecutive buffers to implement the buffer or packet the application is working with? An implementation might need to add another segment for a push operation e.g. if there is not space enough in the current head or tail segment. And it might be useful for ODP implementations to remove unused segments (for pull operations), some HW might not like to operate on empty segments (or indeed any segment with a small number of bytes of data). So I can't understand how ODP can define that push and pull operations shall *not* affect the underlying segmentation of the packet. Signed-off-by: Jerin Jacob jerin.ja...@caviumnetworks.com --- test/validation/buffer/odp_packet_test.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/validation/buffer/odp_packet_test.c b/test/validation/buffer/odp_packet_test.c index b6fa028..7c2b169 100644 --- a/test/validation/buffer/odp_packet_test.c +++ b/test/validation/buffer/odp_packet_test.c @@ -289,6 +289,9 @@ static void _verify_tailroom_shift(odp_packet_t pkt, tail = odp_packet_pull_tail(pkt, -shift); } + seg = odp_packet_last_seg(pkt); + CU_ASSERT(seg != ODP_SEGMENT_INVALID); + CU_ASSERT(tail != NULL); CU_ASSERT(odp_packet_seg_data_len(pkt, seg) == seg_data_len + shift); CU_ASSERT(odp_packet_len(pkt) == pkt_data_len + shift); -- Taras Kondratiuk ___ 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] [PATCH 2/3] validation: buffer: fix for the use of cached return value of odp_packet_last_seg
odp_packet_seg_t is an opaque type, based on the implementation, the return value of odp_packet_last_seg can be changed after headroom/tailroom push/pull operation. Signed-off-by: Jerin Jacob jerin.ja...@caviumnetworks.com --- test/validation/buffer/odp_packet_test.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/validation/buffer/odp_packet_test.c b/test/validation/buffer/odp_packet_test.c index b6fa028..7c2b169 100644 --- a/test/validation/buffer/odp_packet_test.c +++ b/test/validation/buffer/odp_packet_test.c @@ -289,6 +289,9 @@ static void _verify_tailroom_shift(odp_packet_t pkt, tail = odp_packet_pull_tail(pkt, -shift); } + seg = odp_packet_last_seg(pkt); + CU_ASSERT(seg != ODP_SEGMENT_INVALID); + CU_ASSERT(tail != NULL); CU_ASSERT(odp_packet_seg_data_len(pkt, seg) == seg_data_len + shift); CU_ASSERT(odp_packet_len(pkt) == pkt_data_len + shift); -- 1.9.3 ___ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH 2/3] validation: buffer: fix for the use of cached return value of odp_packet_last_seg
On Sat, Jan 17, 2015 at 5:29 AM, Jerin Jacob jerin.ja...@caviumnetworks.com wrote: odp_packet_seg_t is an opaque type, based on the implementation, the return value of odp_packet_last_seg can be changed after headroom/tailroom push/pull operation. Signed-off-by: Jerin Jacob jerin.ja...@caviumnetworks.com Reviewed-by: Bill Fischofer bill.fischo...@linaro.org --- test/validation/buffer/odp_packet_test.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/validation/buffer/odp_packet_test.c b/test/validation/buffer/odp_packet_test.c index b6fa028..7c2b169 100644 --- a/test/validation/buffer/odp_packet_test.c +++ b/test/validation/buffer/odp_packet_test.c @@ -289,6 +289,9 @@ static void _verify_tailroom_shift(odp_packet_t pkt, tail = odp_packet_pull_tail(pkt, -shift); } + seg = odp_packet_last_seg(pkt); + CU_ASSERT(seg != ODP_SEGMENT_INVALID); + CU_ASSERT(tail != NULL); CU_ASSERT(odp_packet_seg_data_len(pkt, seg) == seg_data_len + shift); CU_ASSERT(odp_packet_len(pkt) == pkt_data_len + shift); -- 1.9.3 ___ 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
Re: [lng-odp] [PATCH 2/3] validation: buffer: fix for the use of cached return value of odp_packet_last_seg
On 01/17/2015 01:29 PM, Jerin Jacob wrote: odp_packet_seg_t is an opaque type, based on the implementation, the return value of odp_packet_last_seg can be changed after headroom/tailroom push/pull operation. No. By definition headroom/tailroom push/pull operations don't change segmentation. So the last segment must remain the same. Signed-off-by: Jerin Jacob jerin.ja...@caviumnetworks.com --- test/validation/buffer/odp_packet_test.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/validation/buffer/odp_packet_test.c b/test/validation/buffer/odp_packet_test.c index b6fa028..7c2b169 100644 --- a/test/validation/buffer/odp_packet_test.c +++ b/test/validation/buffer/odp_packet_test.c @@ -289,6 +289,9 @@ static void _verify_tailroom_shift(odp_packet_t pkt, tail = odp_packet_pull_tail(pkt, -shift); } + seg = odp_packet_last_seg(pkt); + CU_ASSERT(seg != ODP_SEGMENT_INVALID); + CU_ASSERT(tail != NULL); CU_ASSERT(odp_packet_seg_data_len(pkt, seg) == seg_data_len + shift); CU_ASSERT(odp_packet_len(pkt) == pkt_data_len + shift); -- Taras Kondratiuk ___ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
