On 7/28/2016 5:27 PM, Jarod Wilson wrote:

On Wed, Jul 27, 2016 at 09:32:34PM -0400, Jarod Wilson wrote:

quoted

I can't see any good reason why inline_hdr and inline_hdr_start should be
two separate arrays in struct mlx5_wqe_eth_seg. The only time I see
anything actually accessed by dereferencing either inline_hdr or
inline_hdr_start is when the MLX5_ETH_L2_INLINE_HEADER_SIZE or less bytes
are copied into inline_hdr_start. By default, it's 18 bytes, copied to
what is a 2-byte and a 16-byte array back to back in the struct, and
coverity and clang both sound alarms because the code says "just write 18
bytes into that 2-byte array", which in practice is ultimately fine, but
again, why?...

I propose to just add two bytes to inline_hdr and drop inline_hdr_start.

Heh, okay, so I see "Add TSO support for RAW Ethernet QP" posted today,
which does actually make use of those bits. Still not sure the split is
actually required though, that patch could probably be reworked to
operate on a single array as well.

The reason for that is that from HW/PRM point of view the 
mlx5_wqe_eth_seg structure defined to include only 2 bytes for the 
inline header start to be 16 bytes structure aligned with its previous 
fields. The extra 16 bytes field was added to ease the implementation 
for copying the extra inline header.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html