[dpdk-dev] [PATCH 01/38] eal: add support for 24 40 and 48 bit operations

Hemant Agrawal hemant.agrawal at nxp.com
Tue Jun 20 12:43:53 CEST 2017

On 6/19/2017 7:22 PM, Wiles, Keith wrote:
>> On Jun 19, 2017, at 6:00 AM, Shreyansh Jain <shreyansh.jain at nxp.com> wrote:
>> Hello Adrien,
>> On Friday 16 June 2017 04:04 PM, Adrien Mazarguil wrote:
>>> Hi Shreyansh,
>>> On Fri, Jun 16, 2017 at 09:21:35AM +0000, Shreyansh Jain wrote:
>>>> Hi Bruce,
>>>>> -----Original Message-----
>>>>> From: Bruce Richardson [mailto:bruce.richardson at intel.com]
>>>>> Sent: Friday, June 16, 2017 2:27 PM
>>>>> To: Shreyansh Jain <shreyansh.jain at nxp.com>
>>>>> Cc: dev at dpdk.org; ferruh.yigit at intel.com; Hemant Agrawal
>>>>> <hemant.agrawal at nxp.com>
>>>>> Subject: Re: [dpdk-dev] [PATCH 01/38] eal: add support for 24 40 and 48 bit
>>>>> operations
>>>>> On Fri, Jun 16, 2017 at 11:10:31AM +0530, Shreyansh Jain wrote:
>>>>>> From: Hemant Agrawal <hemant.agrawal at nxp.com>
>>>>>> Bit Swap and LE<=>BE conversions for 23, 40 and 48 bit width
>>>>>> Signed-off-by: Hemant Agrawal <hemant.agrawal at nxp.com>
>>>>>> ---
>>>>>>  .../common/include/generic/rte_byteorder.h         | 78
>>>>> ++++++++++++++++++++++
>>>>>>  1 file changed, 78 insertions(+)
>>>>> Are these really common enough for inclusion in an generic EAL file?
>>>>> Would they be better being driver specific, so that we don't end up with
>>>>> lots of extra byte-swap routines for each possible size used by a
>>>>> driver.
>>>>  Reasoning was to keep all bit/byte swap at a single place and if it is
>>>> useful for others.
>>>> From DPAA perspective, these macro can be anywhere. In case someone else too
>>>> has use of this (now or in near-future), probably then we can consider this
>>>> in EAL.
>>>> Else, if I don't get much responses in a few days, I will shift them to
>>>> DPAA driver in next version of this series.
>>> While I'm not against exposing exotic byte swapping functions, they are not
>>> completely safe and I'm not sure they should be part of public header files
>>> on that basis.
>>> Problem is their storage size is larger than the number of bytes they deal
>>> with, which raises the question: are filler bytes prepended or appended to
>>> the converted value? How about input values in non-native order? Answering
>>> that is not so easy as it depends on the use case. We actually had a similar
>>> issue when defining VXLAN's VNI field for rte_flow, which is 24-bit in
>>> network order.
>>> Take rte_constant_bswap48() for instance, assuming input value is
>>> little-endian, output is supposed to be big-endian. While the shifts are
>>> correct, filler bytes are not in the right place for a big-endian system,
>>> and the resulting value stored on uint64_t cannot be used as-is. Again, that
>>> depends on the use case, it could be correct if the resulting value was to
>>> be used as is on a little-endian system.
>> I understand what you have stated - the application or any user needs to be context aware about what they are using and the side-effect of such conversions.
>>> I think the only safe way to deal with that is by defining specific types of
>>> the proper size, e.g.:
>>>  typedef uint8_t uint48_t[6];
>>> These are cumbersome and cannot be used like normal integers though. With
>>> such types, byte-swapping functions become meaningless.
>>> Since these are supposed to be rather simple functions, I'm not sure
>>> handling/documenting all this complexity in rte_byteorder.h makes sense.
>> I have no issues moving these into DPAA specific code. Hemant added them in generic just in case they would be of use to others.
>> -
>> Shreyansh
> These are all static inline functions, so no real code increase unless used and having them in one spot is the best place IMO.
> Regards,
> Keith

Yes! these are simple static inline functions with no core unless used.
Many hardware accelerators usages 40 bit & 48 bits data. we thought, it 
can be usable by others as well.

currently we are seeing a mixed opinion.

In next revision, We will move them within our driver code. If someone 
need them in future, we can always bring them to eal.



More information about the dev mailing list