RE: [PATCH] flow-dissector: Fix alignment issue in __skb_flow_get_ports

From: Alexander Duyck

On 10/09/2014 09:47 PM, David Miller wrote:

quoted

From: alexander.duyck@gmail.com
Date: Thu, 09 Oct 2014 21:03:28 -0700

quoted

From: Alexander Duyck <redacted>

This patch addresses a kernel unaligned access bug seen on a sparc64 system
with an igb adapter.  Specifically the __skb_flow_get_ports was returning a
be32 pointer which was then having the value directly returned.

In order to keep the handling of the ports consistent with how we were
handling the IPv4 and IPv6 addresses I have instead replaced the assignment
with a memcpy to the flow key ports value.  This way it should stay a
memcpy on systems that cannot handle unaligned access, and will likely be
converted to a 32b assignment on the systems that can support it.

Reported-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Alexander Duyck <redacted>

Guess what the compiler will output for the memcpy()....

	*(u32 *)dest = *(u32 *)src;

Using memcpy() is never a valid way to avoid misaligned loads and
stores.

If needed we could convert ports to a __be16 I suppose.

You would have to cast it somewhere where the compiler cannot
tell what the original type was.
This usually means you have to call a non-static function, which
might have to be in a different compilation unit.

...

That is what I get for trying to come up with a fix at the end of the
day.  Although it does leave me scratching my head why the IPv4 and IPv6
addresses were not causing unaligned accesses or were they triggering
them as well?

I think there is code to copy the IP and TCP headers to aligned memory
before they are parsed.

...

The problem is the igb / ixgbe / fm10k hardware doesn't have a means of
inserting padding from its side...

Shoot the hardware engineers.

You aren't going to get the performance you expect from a 10Ge card
unless the rx buffers are 'correctly' aligned.

	David