On Fri, Mar 01, 2024 at 05:24:39PM +0000, David Laight wrote: > From: Charlie Jenkins > > Sent: 01 March 2024 17:09 > > > > On Fri, Mar 01, 2024 at 07:17:38AM +0000, Christophe Leroy wrote: > > > +CC netdev ARM Russell > > > > > > Le 29/02/2024 à 23:46, Charlie Jenkins a écrit : > > > > The test cases for ip_fast_csum and csum_ipv6_magic were not properly > > > > aligning the IP header, which were causing failures on architectures > > > > that do not support misaligned accesses like some ARM platforms. To > > > > solve this, align the data along (14 + NET_IP_ALIGN) bytes which is the > > > > standard alignment of an IP header and must be supported by the > > > > architecture. > > > > > > In your description, please provide more details on platforms that have > > > a problem, what the problem is exactly (Failed calculation, slowliness, > > > kernel Oops, panic, ....) on each platform. > > > > > > And please copy maintainers and lists of platforms your are specifically > > > addressing with this change. And as this is network related, netdev list > > > should have been copied as well. > > > > > > I still think that your patch is not the good approach, it looks like > > > you are ignoring all the discussion. Below is a quote of what Geert said > > > and I fully agree with that: > > > > > > IMHO the tests should validate the expected functionality. If a test > > > fails, either functionality is missing or behaves wrong, or the test > > > is wrong. > > > > > > What is the point of writing tests for a core functionality like network > > > checksumming that do not match the expected functionality? > > > > > > > > > So we all agree that there is something to fix, because today's test > > > does odd-address accesses which is unexpected for those functions, but > > > 2-byte alignments should be supported hence tested by the test. Limiting > > > the test to a 16-bytes alignment deeply reduces the usefullness of the test. > > > > > > > Maybe I am lost in the conversations. This isn't limited to 16-bytes > > alignment? It aligns along 14 + NET_IP_ALIGN. That is 16 on some > > platforms and 14 on platforms where unaligned accesses are desired. > > These functions are expected to be called with this offset. Testing with > > any other alignment is not the expected behavior. These tests are > > testing the expected functionality. > > Aligned received frames can have a 4 byte VLAN header (or two) removed. > So the alignment of the IP header is either 4n or 4n+2. > If the cpu fault misaligned accesses you really want the alignment > to be 4n. > > You pretty much never want to trap and fixup a misaligned access. > Especially in the network stack. > I suspect it is better to do a realignment copy of the entire frame. > At some point the data will be copied again, although you may want > a CBU (crystal ball unit) to decide whether to align on an 8n > or 8n+4 boundary to optimise a later copy. > > CPU that support misaligned transfers just make coders sloppy :-) > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) > Can you elaborate on how exactly you suggest the tests to be changed to accomidate what you are saying here? I don't understand how what I have proposed doesn't represent the use case of these functions. - Charlie
