No problem. Yes, pkt.len is the required size of the packet excluding headroom and tailroom, as these are specified separately.
The purpose of pkt.len in odp_pool_params_t is to allow the implementation to calculate the amount of storage it needs to reserve for the pool. The application specifies that it wants a pool large enough to hold pkt.num packets of length pkt.len. How many packets can actually be allocated from the pool may be less than pkt.num if the average packet size exceeds pkt.len, but it will never be more than pkt.num. On Tue, Jul 21, 2015 at 2:44 AM, Nicolas Morey-Chaisemartin < nmo...@kalray.eu> wrote: > Sorry about that. > It works because buffer_alloc will allocate another segment > And because buffer_alloc checks that len + HR + TR <= seg_size * MAX_SEG, > it always works. > > > On 07/21/2015 08:27 AM, Nicolas Morey-Chaisemartin wrote: > > Does that not apply to the len parameter too? > if the user expects to have a buffer of N bytes, shouldn't the internal > buffer size be N + Headroom + Tailroom? > > I don't think this is the case today. > If a user asks for seg_len=1598 and len = 6656 ((1598 + 66 + 0) * 4), he > will end up with a buffer just large enough for its > data but with no headroom at all. > > On 07/21/2015 12:42 AM, Bill Fischofer wrote: > > The tests are just approximations. > > ODP_CONFIG_SEG_LEN_MIN is supposed to be the implementation's minimum > supportable segment size, independent of how that segment is used. We > really want to change all of these from #defines to APIs so that they can > be more portable, but they're #defines for now. > > The seg_len specified on the odp_pool_param_t is supposed to specify the > minimum segment size that the application wishes to see for its data, hence > headroom and tailroom are not part of that. > > On Mon, Jul 20, 2015 at 11:09 AM, Nicolas Morey-Chaisemartin < > <nmo...@kalray.eu>nmo...@kalray.eu> wrote: > >> I've been looking a bit around this patch and something is not clear for >> me about ODP_CONFIG_SEG_LEN_MIN. >> Is is the minimal length of user data that you can fit in a segment, or >> data altogether? >> >> In the current implementation, we round up to SEG_LEN_MIN, and then round >> up again to add head/tail room and align to the right buffer size. >> Is this the expected behavior? >> >> IMHO, it would make sense to use the space added by the round up to >> SEG_LEN_MIN to use for headroom + footer >> >> This would mean replacing the current >> MAX(seg_len, SEG_LEN_MIN) + HEADROOM + FOOTROOM >> by >> MAX(seg_len + HEADROOM + FOOTROOM, SEG_LEN_MIN) >> >> On x86 it probably won't mean much but on restricted memory H/W this can >> make quite a difference. >> >> >> Also, why do we need Headroom + Tail room in each segment and not to the >> total length? >> >> Adding it silently to the total length (still caped by BUF_MAX_LEN) would >> mean that: >> - we use less space on multiple segment packets >> only 1 * (HR+TR) instead of MAX_SEGMENT * (HR+TR) >> - users wouldn't have to do the current trick of subtracting the HR and >> TR from the packet_len when allocating a packet. >> I'm pretty sure 99% of new users will not think about doing this: >> >> static const uint32_t packet_len = PACKET_BUF_LEN - >> ODP_CONFIG_PACKET_HEADROOM - >> ODP_CONFIG_PACKET_TAILROOM - >> PACKET_TAILROOM_RESERVE; >> >> On the down side, it will probably mean that smaller packets are more >> likely to be fragmented but not more than a user would expect when picking >> a seg_len. >> >> Nicolas >> >> On 07/17/2015 01:42 PM, Bill Fischofer wrote: >> >> Nicolas opened this bug and assigned it to me and I see we cross-posted >> fix patches. My patch is at http://patches.opendataplane.org/patch/2298/ >> and is almost the same (I'm using <= rather than < which is slightly more >> efficient for the boundary case where seg_len == >> ODP_CONFIG_PACKET_SEG_LEN_MIN. >> >> On Fri, Jul 17, 2015 at 6:04 AM, Stuart Haslam <stuart.has...@linaro.org> >> wrote: >> >>> On Fri, Jul 17, 2015 at 12:45:52PM +0200, Nicolas Morey-Chaisemartin >>> wrote: >>> > Fixes https://bugs.linaro.org/show_bug.cgi?id=1696 >>> > >>> > Signed-off-by: Nicolas Morey-Chaisemartin < <nmo...@kalray.eu> >>> nmo...@kalray.eu> >>> >>> Reviewed-by: Stuart Haslam < <stuart.has...@linaro.org> >>> stuart.has...@linaro.org> >>> >>> >>> > --- >>> > platform/linux-generic/odp_pool.c | 2 +- >>> > 1 file changed, 1 insertion(+), 1 deletion(-) >>> > >>> > diff --git a/platform/linux-generic/odp_pool.c >>> b/platform/linux-generic/odp_pool.c >>> > index dcbdf07..0f84eb5 100644 >>> > --- a/platform/linux-generic/odp_pool.c >>> > +++ b/platform/linux-generic/odp_pool.c >>> > @@ -205,7 +205,7 @@ odp_pool_t odp_pool_create(const char *name, >>> > tailroom = ODP_CONFIG_PACKET_TAILROOM; >>> > buf_num = params->pkt.num; >>> > >>> > - seg_len = params->pkt.seg_len == 0 ? >>> > + seg_len = params->pkt.seg_len < >>> ODP_CONFIG_PACKET_SEG_LEN_MIN ? >>> > ODP_CONFIG_PACKET_SEG_LEN_MIN : >>> > (params->pkt.seg_len <= >>> ODP_CONFIG_PACKET_SEG_LEN_MAX ? >>> > params->pkt.seg_len : >>> ODP_CONFIG_PACKET_SEG_LEN_MAX); >>> > -- >>> > 2.4.5.3.g4915f6f >>> > >>> > _______________________________________________ >>> > lng-odp mailing list >>> > lng-odp@lists.linaro.org >>> > https://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> -- >>> Stuart. >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> https://lists.linaro.org/mailman/listinfo/lng-odp >>> >> >> >> > > > > _______________________________________________ > lng-odp mailing > listlng-odp@lists.linaro.orghttps://lists.linaro.org/mailman/listinfo/lng-odp > > >
_______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
