On 27 May 2015 at 14:04, Ola Liljedahl <[email protected]> wrote: > On 27 May 2015 at 08:00, Bala Manoharan <[email protected]> wrote: >> >> Hi, >> >> On 26 May 2015 at 20:14, Zoltan Kiss <[email protected]> wrote: >> > >> > >> > On 26/05/15 12:19, Bala Manoharan wrote: >> >> >> >> In the current API the odp_packet_alloc() function always takes "len" >> >> as input parameter and hence it is not required to support any default >> >> size. >> > >> > >> > Yes, that's why I think it's a problem that linux-generic takes 0 as >> > "use >> > the segment size of the pool", whilst another implementation (e.g. >> > ODP-DPDK) >> > can interpret it as a buffer with only (default) headroom and tailroom. >> > I >> > think my interpretation for ODP-DPDK is more logical, but it's not >> > necessarily what we need. We should probably define that in the API. >> >> Yes. we should define this properly in the API but I am not sure about >> the use-case where a 0 size buffer >> will be required by the application. > > I agree it is difficult to see the actual use case for allocating a packet > with length 0 (no data present) but you can also argue that there is nothing > special about a 0 length packet, it should behave the same as when the > length is > 0.
There is nothing wrong in 0 size packet but my concern is that HW limitations will prevent the HW to return a packet of size 0. The packet will return a length of zero but actually the size will be equal to minimal segment size. > > Since this specific value for the size parameter causes such confusion on > what to actually do, I think we should minimum define it better. The > implementation in linux-generic (per above) is not good as the segment size > may change and we don't want that to be directly visible to the application. > > There also needs to be a verification test that when allocating a packet > (using odp_packet_alloc) with data length N, the packet length > (odp_packet_len) is actually N. N includes the value 0 as 0 is a valid > packet data length. I am okay with this test case but I just want to make sure that if a packet of size 0 is allocated from a packet pool the pool will get depleted at the rate equal to the segment size of the pool. Just to be clear if odp_packet_alloc(0) is run 100 times in a loop then the pool will be depleted by an amount of ( 100 * segment size of the pool) . Regards, Bala > > Allocating a packet with length 1 and then calling odp_packet_pull_tail(1) > should create a similar (identical?) 0 length packet. > odp_packet_pull_head(1) has slightly different semantics (the headroom > becomes larger, not the tailroom but this is difficult to observe directly). > >> >> > >> > >> >> In case of odp_buffer_alloc() the default value of allocation will be >> >> equal to the "size" parameter which will be the value given during >> >> odp_pool_create() function as part of odp_pool_params_t variable. >> > >> > Yes, but when you create a packet pool, you'll set up the "pkt" part of >> > that >> > union, so buf.size could be anything. It's just a coincidence that it's >> > pkt.len, odp_buffer_alloc() uses the wrong value there. The same problem >> > applies to odp_packet_alloc(), it also uses buf.size >> >> If the requested pool is of type ODP_POOL_PACKET then implementation >> is supposed to check only the packet union part of the >> odp_pool_param_t as packet pool contains "seg_len" which is the >> minimum continuous data which the applications wants to access in the >> first segment of the packet as this length might indicate the required >> amount of header information to be available in the first segment for >> better performance from application. >> Also a packet pool might do some additional allocation for >> headroom/tailroom and will be different than a simple buffer pool. >> The same holds true for timer pool type also. >> If the above issue is in the linux-generic implementation then we can fix >> that. >> >> > >> >> If this value is given as "0" then the HW will chose the default >> >> segment size supported by the HW. >> >> This value will be the value of the segment with which the pools are >> >> getting created by default in the HW. >> > >> > >> > odp_pool_create just copies the parameters, it doesn't modify them to >> > reflect what values were used in the end. So odp_buffer_alloc() will >> > call >> > buffer_alloc() with s.params.buf.size, and if that's 0, "totsize = >> > pool->s.headroom + size + pool->s.tailroom". >> >> IMO, if the input parameter value is 0 then the implementation should >> store the s.params.buf.size as the >> default segment size supported by the implementation. There might be >> implementations which cannot support >> segments lesser than a default segment size in the system and it might >> always return the default size if the requested value by the >> application is smaller. >> Again if this is an issue in the linux-generic implementation then we >> can fix the same. >> >> Regards, >> Bala >> > >> > Zoli >> > >> > >> >> >> >> Regards, >> >> Bala >> >> >> >> On 26 May 2015 at 16:25, Zoltan Kiss <[email protected]> wrote: >> >>> >> >>> Which value? I've mentioned seg_len and seg_size, but the latter >> >>> actually >> >>> doesn't exist. I guess I meant 'len'. The trouble with them, that you >> >>> can >> >>> set both of them to 0 to ask for the default, so even if you look up >> >>> the >> >>> params with odp_pool_info(), you can end up with both values 0. >> >>> So how do you figure out what length should be used by >> >>> odp_buffer_alloc? >> >>> With the current API definition it will probably end up using pkt.len, >> >>> just >> >>> by accident. >> >>> And the same applies to odp_packet_alloc, although I think it's also a >> >>> bug >> >>> that when the user requested a 0 length buffer (plus headroom and >> >>> tailroom), >> >>> it gets one with the default values. >> >>> >> >>> Zoli >> >>> >> >>> On 23/05/15 03:00, Bill Fischofer wrote: >> >>>> >> >>>> >> >>>> In linux-generic that value represents the default allocation length. >> >>>> >> >>>> On Friday, May 22, 2015, Zoltan Kiss <[email protected] >> >>>> <mailto:[email protected]>> wrote: >> >>>> >> >>>> Hi, >> >>>> >> >>>> While fixing up things in the DPDK implementation I've found >> >>>> that >> >>>> linux-generic might have some troubles too. odp_buffer_alloc() >> >>>> and >> >>>> odp_packet_alloc() uses >> >>>> odp_pool_to_entry(pool_hdl)->s.params.buf.size, but if it's a >> >>>> packet >> >>>> pool (which is always true in case of odp_packet_alloc(), and >> >>>> might >> >>>> be true with odp_buffer_alloc()). >> >>>> My first idea would be to use s.params.pkt.seg_len in that case, >> >>>> but >> >>>> it might be 0. Maybe s.seg_size would be the right value? >> >>>> If anyone has time to come up with a patch to fix this, feel >> >>>> free, >> >>>> I >> >>>> probably won't have time to work on this in the near future. >> >>>> >> >>>> Zoli >> >>>> _______________________________________________ >> >>>> lng-odp mailing list >> >>>> [email protected] >> >>>> https://lists.linaro.org/mailman/listinfo/lng-odp >> >>>> >> >>> _______________________________________________ >> >>> lng-odp mailing list >> >>> [email protected] >> >>> https://lists.linaro.org/mailman/listinfo/lng-odp >> _______________________________________________ >> lng-odp mailing list >> [email protected] >> https://lists.linaro.org/mailman/listinfo/lng-odp > > _______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
