tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Patches fixing unaligned access in the networking code



    Date:        Tue, 28 May 2019 09:14:31 +0200
    From:        Kamil Rytarowski <n54%gmx.com@localhost>
    Message-ID:  <01390f73-c295-5eba-5f95-7c82a75a15ff%gmx.com@localhost>

  | This assert is dummy on x86.

That's OK, x86 has no alignment constraints.   And while unaligned
fetches are indeed slower, as Christos said earlier, aligning an
unaligned data item so it can be fetched (just once) aligned, is even
slower.

Where alignment matters, the field is made to be aligned.

That is, fetching from an unaligned address may be undefined in C,
but it is that way only because on some processors it traps (and on
some weird stupid ones it just fetches bad data).   The kernel code
takes care to make sure an unaligned access does not happen if it
matters.   That is all that is important.

And wrt your previoys message:

  | uint32() is a bad function name as it is a type defined in multiple places
  | in the kernel.

  | I would pick the name get_uint32(). 

OK - except that we don't need it (here at least) at all.

  | I don't like this manual concatenation of bytes and this was rejected
  | previously as prone to timing issues while memcpy() does the right thing. 

I didn't say it was the right way, but that it ought be investigated.
The aim was to have gcc generate the exact same code that it now
generates (no extra moving of bytes or anything) when it is compiling
for an x86 (or other system with no alignment constraints) - that is, I
hoped that it would be smart enough to recognise what the code is doing,
and simply do a 32 bit load.   Which input code pattern would achieve that
(if any, aside from leaving the current code untouched) I did not
investigate.

This code is in the network packet processing fast path - it only exists
at all as an optimisation to make things run faster, since it happens so
much - slowing it down is unacceptable when the reason is nothing more
that satisfying some misapplied technical quibble.

kre



Home | Main Index | Thread Index | Old Index