Paul Mackerras wrote : > Jeff Garzik writes: > > > From: "Philippe De Muyter" <[EMAIL PROTECTED]> > > > > Avoid ppp-generated kernel crashes on machines where unaligned accesses are > > forbidden (ie: 68000-based CPUs) > > This patch seems a bit strange and/or incomplete. Are we trying to > get 2-byte alignment or 4-byte alignment of the payload? It seems
Actually, we try to get a 4n+2 alignment for skb->data, to get the ip-addresses field 4bytes aligned. I think the only thing wrong is the old comment that said : /* Try to get the payload 4-byte aligned */ and that I did not change. > that if the protocol field is uncompressed, we don't do anything to > the alignment, but if it is compressed, we do this: > > > /* protocol is compressed */ > > - skb_push(skb, 1)[0] = 0; > > + if ((unsigned long)skb->data & 1) > > + skb_push(skb, 1)[0] = 0; > > + else { /* Ditto, but realign the payload to 4-byte boundary */ > > + short len = skb->len; > > + > > + skb_put(skb, 3); > > + memmove(skb->data + 3, skb->data, len); > > + skb_pull(skb, 2)[0] = 0; > > I'm puzzled that we are not testing ((unsigned long)skb->data & 2) if > we are really trying to achieve 4-byte alignment. In fact, if the > skb->data that we get from dev_alloc_skb is 4-byte aligned to start > with, this will end up with the payload starting at the original > skb->data + 6, i.e. 2-byte aligned but not 4-byte aligned AFAICS. > > Can we assume that dev_alloc_skb will give us a 4-byte aligned > skb->data? If we can then I suggest we change 3 to 1 in the skb_put Are you not forgetting that the alignment of skb->data is changed (by the existing code ! ) : if (buf[0] != PPP_ALLSTATIONS) skb_reserve(skb, 2 + (buf[0] & 1)); > and memmove above, and get rid of the if (since its condition will > always be false). > > Paul. > Philippe - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html