I think it is more than just naming replacement.
The odp-pool API state that the alignment is for buffer alignment and not for
data. So it seems like a bug, right?
Here is a snipped from the API:
/** Minimum buffer alignment in bytes. Valid values are
powers of two. Use 0 for default alignment.
Default will always be a multiple of 8. */
From: Maxim Uvarov [mailto:maxim.uva...@linaro.org]
Sent: Thursday, August 10, 2017 19:45
To: Liron Himi <lir...@marvell.com>; email@example.com
Cc: Petri Savolainen <petri.savolai...@linaro.org>; Elo, Matias (Nokia -
Subject: [EXT] Re: [lng-odp] ODP1.15 buffer alignment issue
On 08/08/17 09:49, Liron Himi wrote:
> See comments inline
> -----Original Message-----
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
> Maxim Uvarov
> Sent: Monday, August 07, 2017 23:21
> To: firstname.lastname@example.org
> Subject: Re: [lng-odp] ODP1.15 buffer alignment issue
> it's implementation specific.
> Full code is:
> + offset = pool->headroom;
> + /* move to correct align */
> + while (((uintptr_t)&data[offset]) % pool->align != 0)
> + offset++;
> + /* Pointer to data start (of the first segment) */
> + buf_hdr->addr = &data[offset];
> + /* Store buffer into the global pool */
> + ring_enq(ring, mask, (uint32_t)(uintptr_t)buf_hdl);
> If I understood idea right there should be odp specific packet header with
> odp fields which is needed to implement api but missing in hardware buffer
> field. Then hw buffer which is aligned by default with pointer to data.
> Everything depends on implementation and it's not mandatory.
> [L.H.] As part of the linux-generic there is a 'ODP_CONFIG_BUFFER_ALIGN_MIN'
> which should be used for the buffer alignment.
> However, the code above use this value for the data section alignment in the
> buffer and not the buffer address alignment.
> On ODP1.11 the 'ODP_CONFIG_BUFFER_ALIGN_MIN' was used correctly for buffer
> In order to continue we need to understand what was the real intention here:
> 1. To make sure the data section is aligned?
> 2. To make sure the buffer address is aligned? If so, there is a bug in this
> code. A possible fix is as follow:
> - offset = pool->headroom;
> + offset = 0;
> /* move to correct align */
> while (((uintptr_t)&data[offset]) % pool->align != 0)
> + offset += pool->headroom
> /* Pointer to data start (of the first segment) */
> buf_hdr->addr = &data[offset];
> If 1# is required than maybe a 'ODP_CONFIG_DATA_ALIGN_MIN' should be define.
Ah, ok I see. I think in current code we used data align. So that rename to
ODP_CONFIG_DATA_ALIGN_MIN is needed. The remain question is should we also
align buffers itself.
Petri, Matias can you please test with dpdk pktio if buffer alignment improves
> If your platform reuses this linux-generic file and that alignment does not
> work for you then we rework it somehow.
> Bet regards.
> On 08/07/17 21:58, Bill Fischofer wrote:
>> This is part of the latest pool restructure code contributed by Nokia.
>> If you'd like to join the ODP public call tomorrow at 15:00 UTC we
>> can discuss this then.
>> On Mon, Aug 7, 2017 at 8:57 AM, Liron Himi <lir...@marvell.com> wrote:
>>> I'm trying to move to odp1.15 and encounter with an buffer alignment issue.
>>> It seems that linux-generic implementation make sure that the data
>>> address is align with the requested alignment and not the buffer
>>> address itself (look at the code sniped below).
>>> On ODP1.11 (the current version we use) the buffer was aligned correctly.
>>> Is the current behavior is the expected one?
>>> Our HW (and probably other HWs) rely on the fact that the buffers
>>> are aligned.
>>> From odp_pool.c, init_buffers:
>>> offset = pool->headroom;
>>> /* move to correct align */
>>> while (((uintptr_t)&data[offset]) %
>>> pool->align != 0)