On Mon, 21 Aug 2017 09:04:30 +0000, Jan Scheurich wrote:
> The second member of the union should be a variable length array []
> of struct nsh_md2_tlv
>
> struct nsh_hdr {
> ovs_be16 ver_flags_ttl_len;
> uint8_t mdtype;
> uint8_t np;
> ovs_16aligned_be32 path_hdr;
> union {
> struct nsh_md1_ctx md1;
> struct nsh_md2_tlv md2[];
I'm not that sure about this. With each member of md2 having
a different size, you can't use md2 as an array. However, if it was
declared as an array, it might encourage such (wrong) usage.
In particular, nsh_hdr->md2[1] is always wrong.
It seems better to not declare md2 as an array.
> };
> };
>
> That was the original design before Ben removed it due to missing
> support in Microsoft compiler. For the Kernel datapath we should go
> back to that.
>
> I wonder about the possible 16-bit alignment of the 32-bit fields,
> though. How is that handled in the kernel?
get_unaligned_*
> Also struct nsh_md1_ctx
> has 32-bit members, which might not be 32-bit aligned in the packet.
I don't see that happening, it seems the header before md1 is 8 bytes
and sizeof(md1) is 32 bytes? And for md2, the standard mandates that
the md2 size is a multiply of 4 bytes, too.
Jiri
