Re: [PATCH net-next] net/vxlan: Avoid unaligned access in vxlan_build_skb()

[Sowmini Varadhan <sowmini.varadhan@oracle.com>]

On (09/20/16 18:11), Jiri Benc wrote:
> > The vxlan header is at offset (14 + 20 + 8) into the packet,
> > so the vxh is not aligned in vxlan_build_skb. Use [get/put]_unaligned
> > functions to modify flags and vni field in the vxh.
> 
> How did you calculate that? IP header should be aligned to 4 bytes, UDP
> header is 8 bytes, thus VXLAN header is also aligned to 4 bytes.

The vxlan header is after the ethernet header (14 bytes),
IP header (20 bytes, assuming no options) and udp header (8 bytes).
Post the skb_reserve adjustments (see computations in in mld_newpack(),
for example), this triggers an unaligned access on sparc.

> If you went this way, it would be better to make two local variables
> for vx_flags and vx_vni, store to them and do a single put_unaligned
> after the condition. That way, you would have two less put_unaligned and
> no get_unaligned in the remote csum case.

Ok, that is certainly possible.

> And the code would be
> cleaner. And you're missing vx_flags being accessed in
> vxlan_build_gbp_hdr.

Sure, and there's also potential unaligned access in vxlan_build_gpe_hdr.

But as I was telling Tom, this problem is much deeper than this fix, and 
I dont have the facility, at the moment, to test out every one of these
code paths. We would have to fix these, one at a time, in subsequent
patches. This one just fixes the top-level basic code paths.

> But I think this is not needed at all, see above.
> 
>  Jiri