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. 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. 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
