On 9/13/2017 8:26 AM, Nélio Laranjeiro wrote: > On Wed, Sep 13, 2017 at 05:05:14AM +0000, Shahaf Shuler wrote: >> Tuesday, September 12, 2017 9:34 PM, Nélio Laranjeiro: >>>> On Sep 12, 2017, at 12:24 AM, Nélio Laranjeiro >>>>>> Is not it dangerous to assume inl will always be 4 bytes long? Why >>>>>> not writing the real value instead? >>>>> That was for readability of the code and uint32_t will be always >>>>> 4bytes. But for better readability, it should be 'inline_header' >>>>> instead of 'inl' though. I'm also okay with using "4" as well. Which way >>>>> do >>> you prefer? >>>> >>>> I agree on both, I was not clear enough to explain my thought, if for >>>> some reason the inl moves from uint32_t to uint16_t without touching >>>> the sizeof later, it will cause an issue. >>> >>> I tried to change the sizeof but I found that there are more "sizeof(inl)" >>> in the >>> following lines. Changing all the sizeof would be beyond the scope of this >>> patch. So, how about leaving it as is for consistency? >> >> The inline segment format is not expected to change so easily. It is parsed >> by the HW and HW maintains backward compatibility for all of the WQE >> structures. > > We are not talking here about the hardware behavior but about wrong ways to > define hardware values which should be used trough define and not by size of > variable which can be wrongly modified. > If the hardware inline_header size of 4, it should be defined as is. > > I won't block this patch as it is not the single place where this sizeof is > used, but it should be replaced as soon as possible by a define to avoid wrong > behaviors in the future. > > Acked-by: Nelio Laranjeiro <nelio.laranje...@6wind.com>
Applied to dpdk-next-net/master, thanks.
