Re: Misalignment, MIPS, and ip_hdr(skb)->version
David Laightwrites: > From: Måns Rullgård >> Sent: 10 December 2016 13:25 > ... >> I solved this problem in an Ethernet driver by copying the initial part >> of the packet to an aligned skb and appending the remainder using >> skb_add_rx_frag(). The kernel network stack only cares about the >> headers, so the alignment of the packet payload doesn't matter. > > That rather depends on where the packet payload ends up. > It is likely that it will be copied to userspace (or maybe > into some aligned kernel buffer). > In which case you get an expensive misaligned copy. There's not much to be done about that. The only other option is to bypass DMA entirely, and that's sure to be even worse. > What do the hardware engineers think the ethernet interface will > be used for! Ticking boxes in marketing material. -- Måns Rullgård
RE: Misalignment, MIPS, and ip_hdr(skb)->version
From: Måns Rullgård > Sent: 10 December 2016 13:25 ... > I solved this problem in an Ethernet driver by copying the initial part > of the packet to an aligned skb and appending the remainder using > skb_add_rx_frag(). The kernel network stack only cares about the > headers, so the alignment of the packet payload doesn't matter. That rather depends on where the packet payload ends up. It is likely that it will be copied to userspace (or maybe into some aligned kernel buffer). In which case you get an expensive misaligned copy. Encapsulation protocols not using headers that are multiples of 4 bytes is as stupid as ethernet hardware that can't place the mac address on a 4n+2 boundary. The latter is particularly stupid when it happens on embedded silicon with a processor that can only do aligned memory accesses. What do the hardware engineers think the ethernet interface will be used for! David
Re: Misalignment, MIPS, and ip_hdr(skb)->version
On Sun, Dec 11, 2016 at 03:50:31PM +0100, Jason A. Donenfeld wrote: > 3. Add 3 bytes of padding, set to zero, to the encrypted section just > before the IP header, marked for future use. > Pros: satisfies IETF mantras, can use those extra bits in the future > for interesting protocol extensions for authenticated peers. > Cons: lowers MTU, marginally more difficult to implement but still > probably just one or two lines of code. > > Of these, I'm leaning toward (3). Or 4) add one byte to the cleartext header for future use (mostly flags maybe) and 2 bytes of padding to the encrypted header. This way you get the following benefits : 1) your encrypted text is at least 16-bit aligned, maybe it matters in your checksum computations on during decryption 2) your MTU remains even, this is better for both ends 3) you're free to add some bits either to the encrypted or the clear parts. Just a suggestion :-) Willy
Re: Misalignment, MIPS, and ip_hdr(skb)->version
On Sun, Dec 11, 2016 at 4:30 PM, Andrew Lunnwrote: > I'm not a crypto expert, but does this not give you a helping hand in > breaking the crypto? You know the plain text value of these bytes, and > where they are in the encrypted text. You also know with some probability that there's going to be an IP header and a TCP header, each with predictable fields. Maybe you're reasonably certain there's an HTTP header in there too. Gasp! But fear not... Symmetric ciphers are generally not considered secure if they fall to what's called a "known plaintext attack". Fortunately, modern ciphers like AES and ChaCha20 and most others that you're aware of are generally believed to be secure against KPA.
Re: Misalignment, MIPS, and ip_hdr(skb)->version
> 3. Add 3 bytes of padding, set to zero, to the encrypted section just > before the IP header, marked for future use. > Pros: satisfies IETF mantras, can use those extra bits in the future > for interesting protocol extensions for authenticated peers. > Cons: lowers MTU, marginally more difficult to implement but still > probably just one or two lines of code. I'm not a crypto expert, but does this not give you a helping hand in breaking the crypto? You know the plain text value of these bytes, and where they are in the encrypted text. Andrew
Re: Misalignment, MIPS, and ip_hdr(skb)->version
Hey guys, Thanks for the extremely detailed answers. The main take-away from this is that passing unaligned packets to the networking stack kills kittens. So now it's a question of mitigation. I have three options: 1. Copy the plaintext to three bytes before the start of the cipher text, overwriting parts of the header that aren't actually required. Pros: no changes required, MTU stays small. Cons: scatterwalk's fast paths aren't hit, which means two page table mappings are taken instead of one. I have no idea if this actually matters or will slow down anything relavent. 2. Add 3 bytes to the plaintext header, set to zero, marked for future use. Pros: satisfies IETF mantras and makes unaligned in-place decryption straightforward. Cons: lowers MTU, additional unauthenticated cleartext bits in the header are of limited utility in protocol. 3. Add 3 bytes of padding, set to zero, to the encrypted section just before the IP header, marked for future use. Pros: satisfies IETF mantras, can use those extra bits in the future for interesting protocol extensions for authenticated peers. Cons: lowers MTU, marginally more difficult to implement but still probably just one or two lines of code. Of these, I'm leaning toward (3). Anyway, thanks a lot for the input. "Doing nothing" is no longer under serious consideration, thanks to your messages. Thanks, Jason
Re: Misalignment, MIPS, and ip_hdr(skb)->version
Willy Tarreauwrites: > Hi Jason, > > On Thu, Dec 08, 2016 at 11:20:04PM +0100, Jason A. Donenfeld wrote: >> Hi David, >> >> On Thu, Dec 8, 2016 at 1:37 AM, David Miller wrote: >> > You really have to land the IP header on a proper 4 byte boundary. >> > >> > I would suggest pushing 3 dummy garbage bytes of padding at the front >> > or the end of your header. >> >> Are you sure 3 bytes to get 4 byte alignment is really the best? > > It's always the best. However there's another option which should be > considered : maybe it's difficult but not impossible to move some bits > from the current protocol to remove one byte. That's not always easy, > and sometimes you cannot do it just for one bit. However after you run > through this exercise, if you notice there's really no way to shave > this extra byte, you'll realize there's no room left for future > extensions and you'll more easily accept to add 3 empty bytes for > this, typically protocol version, tags, qos or flagss that you'll be > happy to rely on for future versions of your protocol. Always include some way of extending the protocol in the future. A single bit is often enough. Require a value of zero initially, then if you ever want to change anything, setting it to one can indicate whatever you want, including a complete redesign of the header. Alternatively, a one-bit field can indicate the presence of an extended header yet to be defined. Then old software can still make sense of the basic header. -- Måns Rullgård
Re: Misalignment, MIPS, and ip_hdr(skb)->version
Hi Jason, On Thu, Dec 08, 2016 at 11:20:04PM +0100, Jason A. Donenfeld wrote: > Hi David, > > On Thu, Dec 8, 2016 at 1:37 AM, David Millerwrote: > > You really have to land the IP header on a proper 4 byte boundary. > > > > I would suggest pushing 3 dummy garbage bytes of padding at the front > > or the end of your header. > > Are you sure 3 bytes to get 4 byte alignment is really the best? It's always the best. However there's another option which should be considered : maybe it's difficult but not impossible to move some bits from the current protocol to remove one byte. That's not always easy, and sometimes you cannot do it just for one bit. However after you run through this exercise, if you notice there's really no way to shave this extra byte, you'll realize there's no room left for future extensions and you'll more easily accept to add 3 empty bytes for this, typically protocol version, tags, qos or flagss that you'll be happy to rely on for future versions of your protocol. Also while it can feel like you're making your protocol less efficient, keep in mind that these 3 bytes only matter for small packets, and Ethernet already pads small frames to 64 bytes, so in practice any IP packet smaller than 46 bytes will not bring any extra savings. Best regards, Willy
Re: Misalignment, MIPS, and ip_hdr(skb)->version
On Sat, Dec 10, 2016 at 11:18:14PM +0100, Dan Lüdtke wrote: > > > On 8 Dec 2016, at 05:34, Daniel Kahn Gillmorwrote: > > > > On Wed 2016-12-07 19:30:34 -0500, Hannes Frederic Sowa wrote: > >> Your custom protocol should be designed in a way you get an aligned ip > >> header. Most protocols of the IETF follow this mantra and it is always > >> possible to e.g. pad options so you end up on aligned boundaries for the > >> next header. > > > > fwiw, i'm not convinced that "most protocols of the IETF follow this > > mantra". we've had multiple discussions in different protocol groups > > about shaving or bloating by a few bytes here or there in different > > protocols, and i don't think anyone has brought up memory alignment as > > an argument in any of the discussions i've followed. > > > > If the trade-off is between 1 padding byte and 2 byte alignment versus > 3 padding bytes and 4 byte alignment I would definitely opt for 3 > padding bytes. I know how that waste feels like to a protocol > designer, but I think it is worth it. Maybe the padding/reserved will > be useful some day for an additional feature. Note, if you do do this (hint, I think it is a good idea), require that these reserved/pad fields always set to 0 for now, so that no one puts garbage in them and then if you later want to use them, it will be a mess. thanks, greg k-h
Re: Misalignment, MIPS, and ip_hdr(skb)->version
> On 8 Dec 2016, at 05:34, Daniel Kahn Gillmorwrote: > > On Wed 2016-12-07 19:30:34 -0500, Hannes Frederic Sowa wrote: >> Your custom protocol should be designed in a way you get an aligned ip >> header. Most protocols of the IETF follow this mantra and it is always >> possible to e.g. pad options so you end up on aligned boundaries for the >> next header. > > fwiw, i'm not convinced that "most protocols of the IETF follow this > mantra". we've had multiple discussions in different protocol groups > about shaving or bloating by a few bytes here or there in different > protocols, and i don't think anyone has brought up memory alignment as > an argument in any of the discussions i've followed. > If the trade-off is between 1 padding byte and 2 byte alignment versus 3 padding bytes and 4 byte alignment I would definitely opt for 3 padding bytes. I know how that waste feels like to a protocol designer, but I think it is worth it. Maybe the padding/reserved will be useful some day for an additional feature. I remember alignment being discussed and taken very seriously in 6man a couple of times. Often, though, protocol designers did align without much discussion. Implementing unaligned protocols is a pain I've experienced first hand.
Re: Misalignment, MIPS, and ip_hdr(skb)->version
On 2016-12-10 21:32, Måns Rullgård wrote: > Felix Fietkauwrites: > >> On 2016-12-10 14:25, Måns Rullgård wrote: >>> Felix Fietkau writes: >>> On 2016-12-07 19:54, Jason A. Donenfeld wrote: > On Wed, Dec 7, 2016 at 7:51 PM, David Miller wrote: >> It's so much better to analyze properly where the misalignment comes from >> and address it at the source, as we have for various cases that trip up >> Sparc too. > > That's sort of my attitude too, hence starting this thread. Any > pointers you have about this would be most welcome, so as not to > perpetuate what already seems like an issue in other parts of the > stack. Hi Jason, I'm the author of that hackish LEDE/OpenWrt patch that works around the misalignment issues. Here's some context regarding that patch: I intentionally put it in the target specific patches for only one of our MIPS targets. There are a few ar71xx devices where the misalignment cannot be fixed, because the Ethernet MAC has a 4-byte DMA alignment requirement, and does not support inserting 2 bytes of padding to correct the IP header misalignment. With these limitations the choice was between this ugly network stack patch or inserting a very expensive memmove in the data path (which is better than taking the mis-alignment traps, but still hurts routing performance significantly). >>> >>> I solved this problem in an Ethernet driver by copying the initial part >>> of the packet to an aligned skb and appending the remainder using >>> skb_add_rx_frag(). The kernel network stack only cares about the >>> headers, so the alignment of the packet payload doesn't matter. >> >> I considered that as well, but it's bad for routing performance if the >> ethernet MAC does not support scatter/gather for xmit. >> Unfortunately that limitation is quite common on embedded hardware. > > Yes, I can see that being an issue. However, if you're doing zero-copy > routing, the header part of the original buffer should still be there, > unused, so you could presumably copy the header of the outgoing packet > there and then do dma as usual. Maybe there's something in the network > stack that makes this impossible though. That still puts more pressure on the ridiculously small dcache sizes that are typical for embedded MIPS routers. - Felix
Re: Misalignment, MIPS, and ip_hdr(skb)->version
Felix Fietkauwrites: > On 2016-12-10 14:25, Måns Rullgård wrote: >> Felix Fietkau writes: >> >>> On 2016-12-07 19:54, Jason A. Donenfeld wrote: On Wed, Dec 7, 2016 at 7:51 PM, David Miller wrote: > It's so much better to analyze properly where the misalignment comes from > and address it at the source, as we have for various cases that trip up > Sparc too. That's sort of my attitude too, hence starting this thread. Any pointers you have about this would be most welcome, so as not to perpetuate what already seems like an issue in other parts of the stack. >>> Hi Jason, >>> >>> I'm the author of that hackish LEDE/OpenWrt patch that works around the >>> misalignment issues. Here's some context regarding that patch: >>> >>> I intentionally put it in the target specific patches for only one of >>> our MIPS targets. There are a few ar71xx devices where the misalignment >>> cannot be fixed, because the Ethernet MAC has a 4-byte DMA alignment >>> requirement, and does not support inserting 2 bytes of padding to >>> correct the IP header misalignment. >>> >>> With these limitations the choice was between this ugly network stack >>> patch or inserting a very expensive memmove in the data path (which is >>> better than taking the mis-alignment traps, but still hurts routing >>> performance significantly). >> >> I solved this problem in an Ethernet driver by copying the initial part >> of the packet to an aligned skb and appending the remainder using >> skb_add_rx_frag(). The kernel network stack only cares about the >> headers, so the alignment of the packet payload doesn't matter. > > I considered that as well, but it's bad for routing performance if the > ethernet MAC does not support scatter/gather for xmit. > Unfortunately that limitation is quite common on embedded hardware. Yes, I can see that being an issue. However, if you're doing zero-copy routing, the header part of the original buffer should still be there, unused, so you could presumably copy the header of the outgoing packet there and then do dma as usual. Maybe there's something in the network stack that makes this impossible though. -- Måns Rullgård
Re: Misalignment, MIPS, and ip_hdr(skb)->version
On 2016-12-10 14:25, Måns Rullgård wrote: > Felix Fietkauwrites: > >> On 2016-12-07 19:54, Jason A. Donenfeld wrote: >>> On Wed, Dec 7, 2016 at 7:51 PM, David Miller wrote: It's so much better to analyze properly where the misalignment comes from and address it at the source, as we have for various cases that trip up Sparc too. >>> >>> That's sort of my attitude too, hence starting this thread. Any >>> pointers you have about this would be most welcome, so as not to >>> perpetuate what already seems like an issue in other parts of the >>> stack. >> Hi Jason, >> >> I'm the author of that hackish LEDE/OpenWrt patch that works around the >> misalignment issues. Here's some context regarding that patch: >> >> I intentionally put it in the target specific patches for only one of >> our MIPS targets. There are a few ar71xx devices where the misalignment >> cannot be fixed, because the Ethernet MAC has a 4-byte DMA alignment >> requirement, and does not support inserting 2 bytes of padding to >> correct the IP header misalignment. >> >> With these limitations the choice was between this ugly network stack >> patch or inserting a very expensive memmove in the data path (which is >> better than taking the mis-alignment traps, but still hurts routing >> performance significantly). > > I solved this problem in an Ethernet driver by copying the initial part > of the packet to an aligned skb and appending the remainder using > skb_add_rx_frag(). The kernel network stack only cares about the > headers, so the alignment of the packet payload doesn't matter. I considered that as well, but it's bad for routing performance if the ethernet MAC does not support scatter/gather for xmit. Unfortunately that limitation is quite common on embedded hardware. - Felix
Re: Misalignment, MIPS, and ip_hdr(skb)->version
Felix Fietkauwrites: > On 2016-12-07 19:54, Jason A. Donenfeld wrote: >> On Wed, Dec 7, 2016 at 7:51 PM, David Miller wrote: >>> It's so much better to analyze properly where the misalignment comes from >>> and address it at the source, as we have for various cases that trip up >>> Sparc too. >> >> That's sort of my attitude too, hence starting this thread. Any >> pointers you have about this would be most welcome, so as not to >> perpetuate what already seems like an issue in other parts of the >> stack. > Hi Jason, > > I'm the author of that hackish LEDE/OpenWrt patch that works around the > misalignment issues. Here's some context regarding that patch: > > I intentionally put it in the target specific patches for only one of > our MIPS targets. There are a few ar71xx devices where the misalignment > cannot be fixed, because the Ethernet MAC has a 4-byte DMA alignment > requirement, and does not support inserting 2 bytes of padding to > correct the IP header misalignment. > > With these limitations the choice was between this ugly network stack > patch or inserting a very expensive memmove in the data path (which is > better than taking the mis-alignment traps, but still hurts routing > performance significantly). I solved this problem in an Ethernet driver by copying the initial part of the packet to an aligned skb and appending the remainder using skb_add_rx_frag(). The kernel network stack only cares about the headers, so the alignment of the packet payload doesn't matter. -- Måns Rullgård
Re: Misalignment, MIPS, and ip_hdr(skb)->version
On 2016-12-07 19:54, Jason A. Donenfeld wrote: > On Wed, Dec 7, 2016 at 7:51 PM, David Millerwrote: >> It's so much better to analyze properly where the misalignment comes from >> and address it at the source, as we have for various cases that trip up >> Sparc too. > > That's sort of my attitude too, hence starting this thread. Any > pointers you have about this would be most welcome, so as not to > perpetuate what already seems like an issue in other parts of the > stack. Hi Jason, I'm the author of that hackish LEDE/OpenWrt patch that works around the misalignment issues. Here's some context regarding that patch: I intentionally put it in the target specific patches for only one of our MIPS targets. There are a few ar71xx devices where the misalignment cannot be fixed, because the Ethernet MAC has a 4-byte DMA alignment requirement, and does not support inserting 2 bytes of padding to correct the IP header misalignment. With these limitations the choice was between this ugly network stack patch or inserting a very expensive memmove in the data path (which is better than taking the mis-alignment traps, but still hurts routing performance significantly). There are a lot of places in the network stack that assume full 32 bit alignment, and you only get to see those once you start using more of netfilter, play with various tunnel encapsulations, etc. I think you have 3 options to deal with this properly: 1. add 3 bytes of padding 2. allocate a separate skb for decryption (might be more expensive) 3. save the header and decrypt to the start of the packet data (overwriting the misaligned header). I'm not sure what the performance impact of 2 and 3 is, so it's probably best to stick with the padding. I've taken a quick look at the wireguard message headers, and my recommendation would be to insert the 3-byte padding in struct message_header and remove __packed from your structs. This will also remove misaligment of your own protocol fields. - Felix
Re: Misalignment, MIPS, and ip_hdr(skb)->version
On Wed, 07 Dec 2016 23:34:21 -0500, Daniel Kahn Gillmor wrote: > fwiw, i'm not convinced that "most protocols of the IETF follow this > mantra". we've had multiple discussions in different protocol groups > about shaving or bloating by a few bytes here or there in different > protocols, and i don't think anyone has brought up memory alignment as > an argument in any of the discussions i've followed. Which is sad. One would expect that this would be well understood for decades already. Jiri
Re: Misalignment, MIPS, and ip_hdr(skb)->version
From: "Jason A. Donenfeld"Date: Thu, 8 Dec 2016 23:20:04 +0100 > Hi David, > > On Thu, Dec 8, 2016 at 1:37 AM, David Miller wrote: >> You really have to land the IP header on a proper 4 byte boundary. >> >> I would suggest pushing 3 dummy garbage bytes of padding at the front >> or the end of your header. > > Are you sure 3 bytes to get 4 byte alignment is really the best? I was > thinking that adding 1 byte to get 2 byte alignment might be better, > since it would then ensure that the subsequent TCP header winds up > being 4 byte aligned. Or is this in fact not the desired trade off, > and so I should stick with the 3 bytes you suggested? If the IP header is 4 byte aligned, the TCP header will be as well.
Re: Misalignment, MIPS, and ip_hdr(skb)->version
Hi David, On Thu, Dec 8, 2016 at 1:37 AM, David Millerwrote: > You really have to land the IP header on a proper 4 byte boundary. > > I would suggest pushing 3 dummy garbage bytes of padding at the front > or the end of your header. Are you sure 3 bytes to get 4 byte alignment is really the best? I was thinking that adding 1 byte to get 2 byte alignment might be better, since it would then ensure that the subsequent TCP header winds up being 4 byte aligned. Or is this in fact not the desired trade off, and so I should stick with the 3 bytes you suggested? Jason
Re: Misalignment, MIPS, and ip_hdr(skb)->version
On Wed 2016-12-07 19:30:34 -0500, Hannes Frederic Sowa wrote: > Your custom protocol should be designed in a way you get an aligned ip > header. Most protocols of the IETF follow this mantra and it is always > possible to e.g. pad options so you end up on aligned boundaries for the > next header. fwiw, i'm not convinced that "most protocols of the IETF follow this mantra". we've had multiple discussions in different protocol groups about shaving or bloating by a few bytes here or there in different protocols, and i don't think anyone has brought up memory alignment as an argument in any of the discussions i've followed. that said, it sure does sound like it would make things simpler to construct the protocol that way :) --dkg
Re: Misalignment, MIPS, and ip_hdr(skb)->version
From: "Jason A. Donenfeld"Date: Thu, 8 Dec 2016 01:29:42 +0100 > On Wed, Dec 7, 2016 at 8:52 PM, David Miller wrote: >> The only truly difficult case to handle is GRE encapsulation. Is >> that the situation you are running into? >> >> If not, please figure out what the header configuration looks like >> in the case that hits for you, and what the originating device is >> just in case it is a device driver issue. > > My case is my own driver and my own protocol, which uses a 13 byte > header. I can, if absolutely necessary, change the protocol to add > another byte of padding. Or I can choose not to decrypt in place but > rather use a different trick, like overwriting the header during > decryption, though this removes some of the scatterwalk optimizations > when src and dst are the same. Or something else. I wrote the top > email of this thread inquiring about just exactly how bad it is to > call netif_rx(skb) when skb->data is unaligned. You really have to land the IP header on a proper 4 byte boundary. I would suggest pushing 3 dummy garbage bytes of padding at the front or the end of your header.
Re: Misalignment, MIPS, and ip_hdr(skb)->version
On Wed, Dec 7, 2016 at 8:52 PM, David Millerwrote: > The only truly difficult case to handle is GRE encapsulation. Is > that the situation you are running into? > > If not, please figure out what the header configuration looks like > in the case that hits for you, and what the originating device is > just in case it is a device driver issue. My case is my own driver and my own protocol, which uses a 13 byte header. I can, if absolutely necessary, change the protocol to add another byte of padding. Or I can choose not to decrypt in place but rather use a different trick, like overwriting the header during decryption, though this removes some of the scatterwalk optimizations when src and dst are the same. Or something else. I wrote the top email of this thread inquiring about just exactly how bad it is to call netif_rx(skb) when skb->data is unaligned.
Re: Misalignment, MIPS, and ip_hdr(skb)->version
Hi Jason, On 07.12.2016 19:35, Jason A. Donenfeld wrote: > I receive encrypted packets with a 13 byte header. I decrypt the > ciphertext in place, and then discard the header. I then pass the > plaintext to the rest of the networking stack. The plaintext is an IP > packet. Due to the 13 byte header that was discarded, the plaintext > possibly begins at an unaligned location (depending on whether > dev->needed_headroom was respected). > > Does this matter? Is this bad? Will there be a necessary performance hit? Your custom protocol should be designed in a way you get an aligned ip header. Most protocols of the IETF follow this mantra and it is always possible to e.g. pad options so you end up on aligned boundaries for the next header. GRE-TEB for example needs skb_copy_bits to extract the header so it can access them in an aligned way. > In order to find out, I instrumented the MIPS unaligned access > exception handler to see where I was actually in trouble. > Surprisingly, the only part of the stack that seemed to be upset was > on calls to ip_hdr(skb)->version. > > Two things disturb me about this. First, this seems too good to be > true. Does it seem reasonable to you that this is actually the only > place that would be problematic? Or was my testing methodology wrong > to arrive at such an optimistic conclusion? > > Secondly, why should a call to ip_hdr(skb)->version cause an unaligned > access anyway? This struct member is simply the second half of a > single byte in a bit field. I'd expect for the compiler to generate a > single byte load, followed by a bitshift or a mask. Instead, the > compiler appears to generate a double byte load, hence the exception. > What's up with this? Stupid compiler that should be fixed? Some odd > optimization? What to do? I don't see an issue with that at all. Why do you think it could be a problem? Bye, Hannes
Re: Misalignment, MIPS, and ip_hdr(skb)->version
From: "Jason A. Donenfeld"Date: Wed, 7 Dec 2016 19:54:12 +0100 > On Wed, Dec 7, 2016 at 7:51 PM, David Miller wrote: >> It's so much better to analyze properly where the misalignment comes from >> and address it at the source, as we have for various cases that trip up >> Sparc too. > > That's sort of my attitude too, hence starting this thread. Any > pointers you have about this would be most welcome, so as not to > perpetuate what already seems like an issue in other parts of the > stack. The only truly difficult case to handle is GRE encapsulation. Is that the situation you are running into? If not, please figure out what the header configuration looks like in the case that hits for you, and what the originating device is just in case it is a device driver issue. Thanks.
Re: Misalignment, MIPS, and ip_hdr(skb)->version
On Wed, Dec 7, 2016 at 7:51 PM, David Millerwrote: > It's so much better to analyze properly where the misalignment comes from > and address it at the source, as we have for various cases that trip up > Sparc too. That's sort of my attitude too, hence starting this thread. Any pointers you have about this would be most welcome, so as not to perpetuate what already seems like an issue in other parts of the stack.
Re: Misalignment, MIPS, and ip_hdr(skb)->version
From: Dave TahtDate: Wed, 7 Dec 2016 10:47:16 -0800 > https://git.lede-project.org/?p=openwrt/source.git;a=blob;f=target/linux/ar71xx/patches-4.4/910-unaligned_access_hacks.patch;h=b4b749e4b9c02a74a9f712a2740d63e554de5c64;hb=ee53a240ac902dc83209008a2671e7fdcf55957a It's so much better to analyze properly where the misalignment comes from and address it at the source, as we have for various cases that trip up Sparc too. Marking structures "packed" is going to kill performance and is not the answer.
Re: Misalignment, MIPS, and ip_hdr(skb)->version
The openwrt tree has long contained a set of patches that correct for unaligned issues throughout the linux network stack. https://git.lede-project.org/?p=openwrt/source.git;a=blob;f=target/linux/ar71xx/patches-4.4/910-unaligned_access_hacks.patch;h=b4b749e4b9c02a74a9f712a2740d63e554de5c64;hb=ee53a240ac902dc83209008a2671e7fdcf55957a unaligned access traps in the packet processing path on certain versions of the mips architecture is horrifically bad. I had kind of hoped these patches in some form would have made it upstream by now. (or the arches that have the issue retired, I think it's mostly just mips24k)
Misalignment, MIPS, and ip_hdr(skb)->version
Hey MIPS Networking People, I receive encrypted packets with a 13 byte header. I decrypt the ciphertext in place, and then discard the header. I then pass the plaintext to the rest of the networking stack. The plaintext is an IP packet. Due to the 13 byte header that was discarded, the plaintext possibly begins at an unaligned location (depending on whether dev->needed_headroom was respected). Does this matter? Is this bad? Will there be a necessary performance hit? In order to find out, I instrumented the MIPS unaligned access exception handler to see where I was actually in trouble. Surprisingly, the only part of the stack that seemed to be upset was on calls to ip_hdr(skb)->version. Two things disturb me about this. First, this seems too good to be true. Does it seem reasonable to you that this is actually the only place that would be problematic? Or was my testing methodology wrong to arrive at such an optimistic conclusion? Secondly, why should a call to ip_hdr(skb)->version cause an unaligned access anyway? This struct member is simply the second half of a single byte in a bit field. I'd expect for the compiler to generate a single byte load, followed by a bitshift or a mask. Instead, the compiler appears to generate a double byte load, hence the exception. What's up with this? Stupid compiler that should be fixed? Some odd optimization? What to do? I'm considering just adding an extra byte of padding (see discussion in [1]), but before I make any decision like that (and hopefully it won't be necessary), I'd like to completely and entirely understand the full effects and consequences of calling netif_rx(skb) when skb->data is unaligned. Any insight you have to offer would be most welcome. Thanks, Jason [1] https://lists.zx2c4.com/pipermail/wireguard/2016-December/000709.html