Thanks David, On 09/28/2016 09:42 AM, David Laight wrote: > From: Eric Nelson >> Sent: 26 September 2016 19:40 >> Hi David, >> >> On 09/26/2016 02:26 AM, David Laight wrote: >>> From: Eric Nelson >>>> Sent: 24 September 2016 15:42 >>>> The FEC receive accelerator (RACC) supports shifting the data payload of >>>> received packets by 16-bits, which aligns the payload (IP header) on a >>>> 4-byte boundary, which is, if not required, at least strongly suggested >>>> by the Linux networking layer. >>> ... >>>> + /* align IP header */ >>>> + val |= FEC_RACC_SHIFT16; >>> >>> I can't help feeling that there needs to be corresponding >>> changes to increase the buffer size by 2 (maybe for large mtu) >>> and to discard two bytes from the frame length. >>> >> >> In the normal case, the fec driver over-allocates all receive packets to >> be of size FEC_ENET_RX_FRSIZE (2048) minus the value of rx_align, >> which is either 0x0f (ARM) or 0x03 (PPC). >> >> If the frame length is less than rx_copybreak (typically 256), then >> the frame length from the receive buffer descriptor is used to >> control the allocation size for a copied buffer, and this will include >> the two bytes of padding if RACC_SHIFT16 is set. >> >>> If probably ought to be predicated on NET_IP_ALIGN as well. >>> >> Can you elaborate? > > From reading this it seems that the effect of FEC_RACC_SHIFT16 is to > add two bytes of 'junk' to the start of every receive frame. >
That's right. Two bytes of junk between the MAC header and the IP header. > In the 'copybreak' case the new skb would need to be 2 bytes shorter > than the length reported by the hardware, and the data copied from > 2 bytes into the dma buffer. > As it stands, the skb allocated by the copybreak routine will include the two bytes of padding, and the call to skb_pull_inline will ignore them. > The extra 2 bytes also mean the that maximum mtu that can be received > into a buffer is two bytes less. > Right, but I think the max is already high enough that this isn't a problem. > If someone sets the mtu to (say) 9k for jumbo frames this might matter. > Even with fixed 2048 byte buffers it reduces the maximum value the mtu > can be set to by 2. > As far as I can tell, the fec driver doesn't support jumbo frames, and the max frame length is currently hard-coded at PKT_MAXBUF_SIZE (1522). This is well within the 2048-byte allocation, even with optional headers for VLAN etc. > Now if NET_IP_ALIGN is zero then it is fine for the rx frame to start > on a 4n boundary, and the skb are likely to be allocated that way. > In this case you don't want to extra two bytes of 'junk'. > NET_IP_ALIGN is defaulting to 2 by the conditional in skbuff.h > OTOH if NET_IP_ALIGN is 2 then you need to 'fiddle' things so that > the data is dma'd to offset -2 in the skb and then ensure that the > end of frame is set correctly. > That's what the RACC SHIFT16 bit does. The FEC hardware isn't capable of DMA'ing to an un-aligned address. On ARM, it requires 64-bit alignment, but suggests 128-bit alignment. On other (PPC?) architectures, it requires 32-bit alignment. This is handled by the rx_align field. Regards, Eric
