Messages in this thread | | | Date | Mon, 26 Feb 2024 15:48:43 -0800 | Subject | Re: [PATCH v10] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests | From | Guenter Roeck <> |
| |
On 2/26/24 15:17, Charlie Jenkins wrote: > On Mon, Feb 26, 2024 at 10:33:56PM +0000, David Laight wrote: >> ... >>> I think you misunderstand. "NET_IP_ALIGN offset is what the kernel >>> defines to be supported" is a gross misinterpretation. It is not >>> "defined to be supported" at all. It is the _preferred_ alignment >>> nothing more, nothing less. > > This distinction is arbitrary in practice, but I am open to being proven > wrong if you have data to back up this statement. If the driver chooses > to not follow this, then the driver might not work. ARM defines the > NET_IP_ALIGN to be 2 to pad out the header to be on the supported > alignment. If the driver chooses to pad with one byte instead of 2 > bytes, the driver may fail to work as the CPU may stall after the > misaligned access. > >> >> I'm sure I've seen code that would realign IP headers to a 4 byte >> boundary before processing them - but that might not have been in >> Linux. >> >> I'm also sure there are cpu which will fault double length misaligned >> memory transfers - which might be used to marginally speed up code. >> Assuming more than 4 byte alignment for the IP header is likely >> 'wishful thinking'. >> >> There is plenty of ethernet hardware that can only write frames >> to even boundaries and plenty of cpu that fault misaligned accesses. >> There are even cases of both on the same silicon die. >> >> You also pretty much never want a fault handler to fixup misaligned >> ethernet frames (or really anything else for that matter). >> It is always going to be better to check in the code itself. >> >> x86 has just made people 'sloppy' :-) >> >> David >> >> - >> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK >> Registration No: 1397386 (Wales) >> > > If somebody has a solution they deem to be better, I am happy to change > this test case. Otherwise, I would appreciate a maintainer resolving > this discussion and apply this fix. > Agreed.
I do have a couple of patches which add explicit unaligned tests as well as corner case tests (which are intended to trigger as many carry overflows as possible). Once I get those working reliably, I'll be happy to submit them as additional tests.
Thanks, Guenter
| |