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> 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/> > 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 list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
