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/
-
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/