Messages in this thread Patch in this message | | | Date | Thu, 28 Oct 1999 22:30:44 +0200 | From | Henner Eisen <> | Subject | PATCH: isdn_ppp_skb_push:under (was: Re: 2.2.13 ISDN funnies) |
| |
>>>>> "Alan" == Alan Cox <alan@lxorguk.ukuu.org.uk> writes:
Alan> It could be a compressor bug, that is the obvious Alan> candidate. Do people see it with all the compression Alan> disabled ?
Yes, I've already tracked it down now and it was compression related:
The problem is that compression needs to transfer the data to a new skb because a net device is not allowed to modify the data part above skb->nh.raw of the original skb. But the code which allocates the new skb fails to allocate and reserve enough header space. The VJ compression stuff accounts for the HL driver's header space requirements but forgets to account for the additional space needed for the PPP headers. The link compression code (in isdn_ppp_compress()) does not acoount for any header space at all!
The following patch fixes this (tested, works sucesssfully). For the VJ related skb allocation, it simply additionally accounts for the PPP header space. For the generic link compression (in isdn_ppp_compress()) it simply reserves the same headroom as was present in the original skb. (This is not totally bullet proof, see the new comment in the patch and my previous article on the list regarding loading HL drivers after the netdevice was registered. But for reasonable real-world configurations, the problem it will completly disappeer.)
Actually, prior to 2.2.13, the header space calculation for VJ was even more buggy. It did not account for any header space, but as it used dev_alloc_skb(), the implicitly allocated header space was ususally larger than the more correct one in 2.2.13. Thus, the problem is more likly to occur in 2.2.13. The smaller the ppp header, the less likly is the error to occur. If you enabable all ppp features (e.g. mppp, vj, ccp, ..), and your peer agrees to use them, ppp headers will become larger and the problem is more likly to occur. If you disable compression completely, the problem will disappear completely.
Henner
--- linux-2.2.13-orig/drivers/isdn/isdn_ppp.c Thu Oct 28 19:07:06 1999 +++ linux-2.2.13/drivers/isdn/isdn_ppp.c Thu Oct 28 21:16:48 1999 @@ -1540,7 +1540,13 @@ * sk_buff. old call to dev_alloc_skb only reserved * 16 bytes, now we are looking what the driver want. */ - hl = dev->drv[lp->isdn_device]->interface->hl_hdrlen; + hl = dev->drv[lp->isdn_device]->interface->hl_hdrlen + IPPP_MAX_HEADER; + /* + * Note: hl might still be insufficient because the method + * above does not account for a possibible MPPP slave channel + * which had larger HL header space requirements than the + * master. + */ new_skb = alloc_skb(hl+skb->len, GFP_ATOMIC); if (new_skb) { u_char *buf; @@ -2667,9 +2673,10 @@ } /* Allow for at least 150 % expansion (for now) */ - skb_out = dev_alloc_skb(skb_in->len + skb_in->len/2 + 32); + skb_out = dev_alloc_skb(skb_in->len + skb_in->len/2 + 32 + skb_headroom(skb_in)); if(!skb_out) return skb_in; + skb_reserve(skb_out, skb_headroom(skb_in)); ret = (compressor->compress)(stat,skb_in,skb_out,*proto); if(!ret) { - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |