The idea behind odp_packet_alloc(pool, 0) is to allocate an "empty" packet that will then be constructed via successive odp_packet_push_tail() calls to sweep out the number of bytes needed for various headers, etc. as it's being constructed by the the app. Since we don't architecturally support extending the actual allocated length of the packet (just manipulating headroom/tailroom pointers) the question is how much space should be reserved in this case? In the case of linux-generic, for packets this is the pkt.len field specified on the odp_pool_param_t passed to the odp_pool_create() for the pool in question.
On Wed, May 27, 2015 at 4:25 AM, Bala Manoharan <[email protected]> wrote: > 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 >
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
