On 08/08/17 09:49, Liron Himi wrote:
> Hi,
> 
> See comments inline
> 
> Regards,
> Liron
> 
> -----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: lng-odp@lists.linaro.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++;
> <skip>
> +
> +               /* Pointer to data start (of the first segment) */
> +               buf_hdr->addr[0] = &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 
> alignment.
> 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++;
>       +       offset += pool->headroom
>               <skip>
>               /* Pointer to data start (of the first segment) */
>                      buf_hdr->addr[0] = &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 performance?

Maxim.



> If your platform reuses this linux-generic file and that alignment does not 
> work for you then we rework it somehow.
> 
> Bet regards.
> Maxim.
> 
> 
> 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:
>>
>>> Hi,
>>>
>>> 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)
>>>                                                 offset++;
>>>
>>>
>>> Regards,
>>> Liron
>>>
> 

Reply via email to